Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1456005 > unrolled thread
| Started by | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| First post | 2016-08-03 22:00 +0200 |
| Last post | 2016-08-04 11:50 +0200 |
| Articles | 6 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: net: vrf: Handle ipv6 multicast and link-local addresses Geert Uytterhoeven <geert@linux-m68k.org> - 2016-08-03 22:00 +0200
Re: net: vrf: Handle ipv6 multicast and link-local addresses David Ahern <dsa@cumulusnetworks.com> - 2016-08-03 22:20 +0200
Re: net: vrf: Handle ipv6 multicast and link-local addresses Geert Uytterhoeven <geert@linux-m68k.org> - 2016-08-03 22:30 +0200
Re: net: vrf: Handle ipv6 multicast and link-local addresses David Ahern <dsa@cumulusnetworks.com> - 2016-08-03 22:40 +0200
Re: net: vrf: Handle ipv6 multicast and link-local addresses Geert Uytterhoeven <geert@linux-m68k.org> - 2016-08-03 23:10 +0200
Re: net: vrf: Handle ipv6 multicast and link-local addresses Geert Uytterhoeven <geert@linux-m68k.org> - 2016-08-04 11:50 +0200
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2016-08-03 22:00 +0200 |
| Subject | Re: net: vrf: Handle ipv6 multicast and link-local addresses |
| Message-ID | <s2auB-715-11@gated-at.bofh.it> |
On Wed, Jul 27, 2016 at 11:52 PM, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Web: https://git.kernel.org/torvalds/c/9ff74384600aeecba34ebdacbbde0627489ff601
> Commit: 9ff74384600aeecba34ebdacbbde0627489ff601
> Parent: ba46ee4c0ed122fa14aa2f5d6994c166a01ae2c0
> Refname: refs/heads/master
> Author: David Ahern <dsa@cumulusnetworks.com>
> AuthorDate: Mon Jun 13 13:44:19 2016 -0700
> Committer: David S. Miller <davem@davemloft.net>
> 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 <dsa@cumulusnetworks.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> 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
[toc] | [next] | [standalone]
| From | David Ahern <dsa@cumulusnetworks.com> |
|---|---|
| Date | 2016-08-03 22:20 +0200 |
| Message-ID | <s2aNY-7my-5@gated-at.bofh.it> |
| In reply to | #1456005 |
On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
>> +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.
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
>
>> > + .flowi6_mark = skb->mark,
>> > + .flowi6_proto = iph->nexthdr,
>> > + .flowi6_iif = ifindex,
>> > + };
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2016-08-03 22:30 +0200 |
| Message-ID | <s2aXE-7pL-9@gated-at.bofh.it> |
| In reply to | #1456008 |
On Wed, Aug 3, 2016 at 10:11 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
>>> +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.
>
> 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
[toc] | [prev] | [next] | [standalone]
| From | David Ahern <dsa@cumulusnetworks.com> |
|---|---|
| Date | 2016-08-03 22:40 +0200 |
| Message-ID | <s2b7k-7uo-15@gated-at.bofh.it> |
| In reply to | #1456013 |
On 8/3/16 2:27 PM, Geert Uytterhoeven wrote:
> On Wed, Aug 3, 2016 at 10:11 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
>> On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
>>>> +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.
>>
>> 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?
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2016-08-03 23:10 +0200 |
| Message-ID | <s2bAl-7Up-3@gated-at.bofh.it> |
| In reply to | #1456018 |
On Wed, Aug 3, 2016 at 10:36 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 8/3/16 2:27 PM, Geert Uytterhoeven wrote:
>> On Wed, Aug 3, 2016 at 10:11 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
>>> On 8/3/16 1:57 PM, Geert Uytterhoeven wrote:
>>>>> +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.
>>>
>>> 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
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2016-08-04 11:50 +0200 |
| Message-ID | <s2nrQ-76H-3@gated-at.bofh.it> |
| In reply to | #1456027 |
Hi David,
On Thu, Aug 4, 2016 at 7:01 AM, David Ahern <dsa@cumulusnetworks.com> wrote:
> Does making the code the same between those 2 functions matter?
Yes, it does make the warning go away.
> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
> index 1ce7420322ee..3951a2c98431 100644
> --- a/drivers/net/vrf.c
> +++ b/drivers/net/vrf.c
> @@ -862,15 +862,17 @@ 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 net *net = dev_net(vrf_dev);
> struct flowi6 fl6 = {
> + .flowi6_oif = 0,
> + .flowi6_iif = ifindex,
> .daddr = iph->daddr,
> .saddr = iph->saddr,
> .flowlabel = ip6_flowinfo(iph),
> .flowi6_mark = skb->mark,
> .flowi6_proto = iph->nexthdr,
> - .flowi6_iif = ifindex,
> + .flowi6_flags = 0,
> };
> - struct net *net = dev_net(vrf_dev);
> struct rt6_info *rt6;
The critical change seems to be moving the initialization of
.flowi6_iif: if that
is done before the initialization of .flowlabel, there's no compiler warning.
The generated asm output is identical, though, so I think this is some sort of
false positive or compiler bug. Hence let's ignore it.
Thanks!
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web