Re: net: vrf: Handle ipv6 multicast and link-local addresses

6 messages, 3 authors, 2016-10-03 · open the first message on its own page

Re: net: vrf: Handle ipv6 multicast and link-local addresses

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2016-08-03 19:58:29

On Wed, Jul 27, 2016 at 11:52 PM, Linux Kernel Mailing List
[off-list ref] wrote:
quoted hunk
Web:        https://git.kernel.org/torvalds/c/9ff74384600aeecba34ebdacbbde0627489ff601
Commit:     9ff74384600aeecba34ebdacbbde0627489ff601
Parent:     ba46ee4c0ed122fa14aa2f5d6994c166a01ae2c0
Refname:    refs/heads/master
Author:     David Ahern [off-list ref]
AuthorDate: Mon Jun 13 13:44:19 2016 -0700
Committer:  David S. Miller [off-list ref]
CommitDate: Wed Jun 15 12:34:34 2016 -0700

    net: vrf: Handle ipv6 multicast and link-local addresses

    IPv6 multicast and link-local addresses require special handling by the
    VRF driver:
    1. Rather than using the VRF device index and full FIB lookups,
       packets to/from these addresses should use direct FIB lookups based on
       the VRF device table.

    2. fail sends/receives on a VRF device to/from a multicast address
       (e.g, make ping6 ff02::1%<vrf> fail)

    3. move the setting of the flow oif to the first dst lookup and revert
       the change in icmpv6_echo_reply made in ca254490c8dfd ("net: Add VRF
       support to IPv6 stack"). Linklocal/mcast addresses require use of the
       skb->dev.

    With this change connections into and out of a VRF enslaved device work
    for multicast and link-local addresses work (icmp, tcp, and udp)
    e.g.,

    1. packets into VM with VRF config:
        ping6 -c3 fe80::e0:f9ff:fe1c:b974%br1
        ping6 -c3 ff02::1%br1

        ssh -6 fe80::e0:f9ff:fe1c:b974%br1

    2. packets going out a VRF enslaved device:
        ping6 -c3 fe80::18f8:83ff:fe4b:7a2e%eth1
        ping6 -c3 ff02::1%eth1
        ssh -6 root@fe80::18f8:83ff:fe4b:7a2e%eth1

    Signed-off-by: David Ahern [off-list ref]
    Signed-off-by: David S. Miller [off-list ref]
---
 drivers/net/vrf.c       | 98 ++++++++++++++++++++++++++++++++++++++++++++++---
 include/net/ip6_route.h |  2 +
 net/ipv6/icmp.c         |  2 +-
 net/ipv6/route.c        |  5 ++-
 4 files changed, 99 insertions(+), 8 deletions(-)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index d2ce76c..0b5b3c2 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -785,9 +785,63 @@ out:
        return rc;
 }

+static struct rt6_info *vrf_ip6_route_lookup(struct net *net,
+                                            const struct net_device *dev,
+                                            struct flowi6 *fl6,
+                                            int ifindex,
+                                            int flags)
+{
+       struct net_vrf *vrf = netdev_priv(dev);
+       struct fib6_table *table = NULL;
+       struct rt6_info *rt6;
+
+       rcu_read_lock();
+
+       /* fib6_table does not have a refcnt and can not be freed */
+       rt6 = rcu_dereference(vrf->rt6);
+       if (likely(rt6))
+               table = rt6->rt6i_table;
+
+       rcu_read_unlock();
+
+       if (!table)
+               return NULL;
+
+       return ip6_pol_route(net, table, ifindex, fl6, flags);
+}
+
+static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
+                             int ifindex)
+{
+       const struct ipv6hdr *iph = ipv6_hdr(skb);
+       struct flowi6 fl6 = {
+               .daddr          = iph->daddr,
+               .saddr          = iph->saddr,
+               .flowlabel      = ip6_flowinfo(iph),
The above assignment causes the following compiler warning with
m68k-linux-gnu-gcc-4.1:

    drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
    drivers/net/vrf.c:870: warning: initialized field with
side-effects overwritten
    drivers/net/vrf.c:870: warning: (near initialization for ‘fl6’)

Unfortunately I have no idea what it means, nor do I see what's wrong
with the code.
+               .flowi6_mark    = skb->mark,
+               .flowi6_proto   = iph->nexthdr,
+               .flowi6_iif     = ifindex,
+       };
Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: net: vrf: Handle ipv6 multicast and link-local addresses

From: David Ahern <hidden>
Date: 2016-08-03 20:17:07

On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
quoted
+static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
quoted
+                             int ifindex)
+{
+       const struct ipv6hdr *iph = ipv6_hdr(skb);
+       struct flowi6 fl6 = {
+               .daddr          = iph->daddr,
+               .saddr          = iph->saddr,
+               .flowlabel      = ip6_flowinfo(iph),
The above assignment causes the following compiler warning with
m68k-linux-gnu-gcc-4.1:

    drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
    drivers/net/vrf.c:870: warning: initialized field with
side-effects overwritten
    drivers/net/vrf.c:870: warning: (near initialization for ‘fl6’)

Unfortunately I have no idea what it means, nor do I see what's wrong
with the code.
no idea. Fields are initialized once and left and right data types are the same.

Can you remove one line at a time? Line 870 is ".flowi6_proto   = iph->nexthdr," but all of the flowi6 macros are unique references to unique fields in flowi_common. The flowlabel line you point out is a unique field as well.

Can you run pahole on file that did compile? e.g.,

pahole -C 'flowi6' net/ipv6/route.o

and get the common struct too:

pahole -C 'flowi_common' net/ipv6/route.o
quoted
quoted
+               .flowi6_mark    = skb->mark,
+               .flowi6_proto   = iph->nexthdr,
+               .flowi6_iif     = ifindex,
+       };

Re: net: vrf: Handle ipv6 multicast and link-local addresses

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2016-08-03 20:29:34

On Wed, Aug 3, 2016 at 10:11 PM, David Ahern [off-list ref] wrote:
On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
quoted
quoted
+static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
quoted
+                             int ifindex)
+{
+       const struct ipv6hdr *iph = ipv6_hdr(skb);
+       struct flowi6 fl6 = {
+               .daddr          = iph->daddr,
+               .saddr          = iph->saddr,
+               .flowlabel      = ip6_flowinfo(iph),
The above assignment causes the following compiler warning with
m68k-linux-gnu-gcc-4.1:

    drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
    drivers/net/vrf.c:870: warning: initialized field with
side-effects overwritten
    drivers/net/vrf.c:870: warning: (near initialization for ‘fl6’)

Unfortunately I have no idea what it means, nor do I see what's wrong
with the code.
no idea. Fields are initialized once and left and right data types are the same.

Can you remove one line at a time? Line 870 is ".flowi6_proto   = iph->nexthdr," but all of the flowi6 macros are unique references to unique fields in flowi_common. The flowlabel line you point out is a unique field as well.
The only thing that seems to matter is assigning the result of the call to
ip6_flowinfo() to .flowlabel. Assigning a constant makes the warning go away.

Yeah, the 870 line number is funny, as it doesn't point to the offending line.
Can you run pahole on file that did compile? e.g.,

pahole -C 'flowi6' net/ipv6/route.o

and get the common struct too:

pahole -C 'flowi_common' net/ipv6/route.o
No output. Perhaps pahole doesn't play well with cross-compiling?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: net: vrf: Handle ipv6 multicast and link-local addresses

From: David Ahern <hidden>
Date: 2016-08-03 20:37:26

On 8/3/16 2:27 PM, Geert Uytterhoeven wrote:
On Wed, Aug 3, 2016 at 10:11 PM, David Ahern [off-list ref] wrote:
quoted
On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
quoted
quoted
+static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
quoted
+                             int ifindex)
+{
+       const struct ipv6hdr *iph = ipv6_hdr(skb);
+       struct flowi6 fl6 = {
+               .daddr          = iph->daddr,
+               .saddr          = iph->saddr,
+               .flowlabel      = ip6_flowinfo(iph),
The above assignment causes the following compiler warning with
m68k-linux-gnu-gcc-4.1:

    drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
    drivers/net/vrf.c:870: warning: initialized field with
side-effects overwritten
    drivers/net/vrf.c:870: warning: (near initialization for ‘fl6’)

Unfortunately I have no idea what it means, nor do I see what's wrong
with the code.
no idea. Fields are initialized once and left and right data types are the same.

Can you remove one line at a time? Line 870 is ".flowi6_proto   = iph->nexthdr," but all of the flowi6 macros are unique references to unique fields in flowi_common. The flowlabel line you point out is a unique field as well.
The only thing that seems to matter is assigning the result of the call to
ip6_flowinfo() to .flowlabel. Assigning a constant makes the warning go away.
No complaints for the same initialization style at line 151?

Re: net: vrf: Handle ipv6 multicast and link-local addresses

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2016-08-03 21:08:36

On Wed, Aug 3, 2016 at 10:36 PM, David Ahern [off-list ref] wrote:
On 8/3/16 2:27 PM, Geert Uytterhoeven wrote:
quoted
On Wed, Aug 3, 2016 at 10:11 PM, David Ahern [off-list ref] wrote:
quoted
On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
quoted
quoted
+static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
quoted
+                             int ifindex)
+{
+       const struct ipv6hdr *iph = ipv6_hdr(skb);
+       struct flowi6 fl6 = {
+               .daddr          = iph->daddr,
+               .saddr          = iph->saddr,
+               .flowlabel      = ip6_flowinfo(iph),
The above assignment causes the following compiler warning with
m68k-linux-gnu-gcc-4.1:

    drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
    drivers/net/vrf.c:870: warning: initialized field with
side-effects overwritten
    drivers/net/vrf.c:870: warning: (near initialization for ‘fl6’)

Unfortunately I have no idea what it means, nor do I see what's wrong
with the code.
no idea. Fields are initialized once and left and right data types are the same.

Can you remove one line at a time? Line 870 is ".flowi6_proto   = iph->nexthdr," but all of the flowi6 macros are unique references to unique fields in flowi_common. The flowlabel line you point out is a unique field as well.
The only thing that seems to matter is assigning the result of the call to
ip6_flowinfo() to .flowlabel. Assigning a constant makes the warning go away.
No complaints for the same initialization style at line 151?
No.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: net: vrf: Handle ipv6 multicast and link-local addresses

From: Arnaldo Carvalho de Melo <acme@kernel.org>
Date: 2016-10-03 20:00:17

Em Wed, Aug 03, 2016 at 10:27:44PM +0200, Geert Uytterhoeven escreveu:
On Wed, Aug 3, 2016 at 10:11 PM, David Ahern [off-list ref] wrote:
quoted
On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
quoted
quoted
+static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
quoted
+                             int ifindex)
+{
+       const struct ipv6hdr *iph = ipv6_hdr(skb);
+       struct flowi6 fl6 = {
+               .daddr          = iph->daddr,
+               .saddr          = iph->saddr,
+               .flowlabel      = ip6_flowinfo(iph),
The above assignment causes the following compiler warning with
m68k-linux-gnu-gcc-4.1:

    drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
    drivers/net/vrf.c:870: warning: initialized field with
side-effects overwritten
    drivers/net/vrf.c:870: warning: (near initialization for ‘fl6’)

Unfortunately I have no idea what it means, nor do I see what's wrong
with the code.
no idea. Fields are initialized once and left and right data types are the same.

Can you remove one line at a time? Line 870 is ".flowi6_proto   = iph->nexthdr," but all of the flowi6 macros are unique references to unique fields in flowi_common. The flowlabel line you point out is a unique field as well.
The only thing that seems to matter is assigning the result of the call to
ip6_flowinfo() to .flowlabel. Assigning a constant makes the warning go away.

Yeah, the 870 line number is funny, as it doesn't point to the offending line.
quoted
Can you run pahole on file that did compile? e.g.,

pahole -C 'flowi6' net/ipv6/route.o

and get the common struct too:

pahole -C 'flowi_common' net/ipv6/route.o
No output. Perhaps pahole doesn't play well with cross-compiling?
Can you send me this net/ipv6/route.o file? It should work as long as
CONFIG_DEBUG_INFO was used to build this cross kernel.

For instance, here cross compiling perf tools for s390 from a ubuntu
16.04 container running on a fedora 24 host, I get:

[root@jouet 14.04.4]# file /var/lib/docker/devicemapper/mnt/b7ba22999d88feebc42de2ea17fe27e94febf228439fccf2b84c0c9e1ccd8af7/rootfs/tmp/build/perf/perf
/var/lib/docker/devicemapper/mnt/b7ba22999d88feebc42de2ea17fe27e94febf228439fccf2b84c0c9e1ccd8af7/rootfs/tmp/build/perf/perf: ELF 64-bit MSB shared object, IBM S/390, version 1 (SYSV), dynamically linked, interpreter /lib/ld64.so.1, for GNU/Linux 3.2.0, BuildID[sha1]=22b27009c61130ed5727bcaf9ad0813d414b8c5a, not stripped
[root@jouet 14.04.4]#

Thus a s/390 object, that I then process from a x86_64 pahole:

[root@jouet 14.04.4]# file /home/acme/git/pahole/build/pahole
/home/acme/git/pahole/build/pahole: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32, BuildID[sha1]=c90855d4ffa815bf0dddb3463bc45aea4855d0e2, not stripped
[root@jouet 14.04.4]# 

Giving these results:

[root@jouet 14.04.4]# pahole -C sockaddr_in6 /var/lib/docker/devicemapper/mnt/b7ba22999d88feebc42de2ea17fe27e94febf228439fccf2b84c0c9e1ccd8af7/rootfs/tmp/build/perf/perf
struct sockaddr_in6 {
	sa_family_t                sin6_family;          /*     0     2 */
	in_port_t                  sin6_port;            /*     2     2 */
	uint32_t                   sin6_flowinfo;        /*     4     4 */
	struct in6_addr            sin6_addr;            /*     8    16 */
	uint32_t                   sin6_scope_id;        /*    24     4 */

	/* size: 28, cachelines: 1, members: 5 */
	/* last cacheline: 28 bytes */
};
[root@jouet 14.04.4]#

Expanding types:

[root@jouet 14.04.4]# pahole --expand_types -C sockaddr_in6 /var/lib/docker/devicemapper/mnt/b7ba22999d88feebc42de2ea17fe27e94febf228439fccf2b84c0c9e1ccd8af7/rootfs/tmp/build/perf/perf
struct sockaddr_in6 {
	/* typedef sa_family_t */ short unsigned int         sin6_family;                /*     0     2 */
	/* typedef in_port_t -> uint16_t */ short unsigned int         sin6_port;        /*     2     2 */
	/* typedef uint32_t */ unsigned int               sin6_flowinfo;                 /*     4     4 */
	struct in6_addr {
		union {
			/* typedef uint8_t */ unsigned char __u6_addr8[16];              /*          16 */
			/* typedef uint16_t */ short unsigned int __u6_addr16[8];        /*          16 */
			/* typedef uint32_t */ unsigned int __u6_addr32[4];              /*          16 */
		} __in6_u;                                                               /*     8    16 */
	} sin6_addr; /*     8    16 */
	/* typedef uint32_t */ unsigned int               sin6_scope_id;                 /*    24     4 */

	/* size: 28, cachelines: 1, members: 5 */
	/* last cacheline: 28 bytes */
};
[root@jouet 14.04.4]#

etc.

Guess I need to put together a m68k target for building the perf tools (and
test pahole & friends on such objects)... :-)

- Arnaldo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help