| Message ID | 20190624140109.14775-2-nicolas.dichtel@6wind.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipv6: fix neighbour resolution with raw socket | expand |
On Mon, Jun 24, 2019 at 7:01 AM Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote: > > There is no functional change in this patch, it only prepares the next one. > > rt6_nexthop() will be used by ip6_dst_lookup_neigh(), which uses const > variables. > > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Also, I think this fixes an issues reported by 0day: https://groups.google.com/forum/#!searchin/clang-built-linux/const%7Csort:date/clang-built-linux/umkS84jS9m8/GAVVEgNYBgAJ Reported-by: kbuild test robot <lkp@intel.com> Acked-by: Nick Desaulniers <ndesaulniers@google.com> > --- > drivers/net/vrf.c | 2 +- > include/net/ip6_route.h | 4 ++-- > net/bluetooth/6lowpan.c | 4 ++-- > net/ipv6/ip6_output.c | 2 +- > net/netfilter/nf_flow_table_ip.c | 2 +- > 5 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c > index 11b9525dff27..311b0cc6eb98 100644 > --- a/drivers/net/vrf.c > +++ b/drivers/net/vrf.c > @@ -350,8 +350,8 @@ static int vrf_finish_output6(struct net *net, struct sock *sk, > { > struct dst_entry *dst = skb_dst(skb); > struct net_device *dev = dst->dev; > + const struct in6_addr *nexthop; > struct neighbour *neigh; > - struct in6_addr *nexthop; > int ret; > > nf_reset(skb); > diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h > index 4790beaa86e0..ee7405e759ba 100644 > --- a/include/net/ip6_route.h > +++ b/include/net/ip6_route.h > @@ -262,8 +262,8 @@ static inline bool ip6_sk_ignore_df(const struct sock *sk) > inet6_sk(sk)->pmtudisc == IPV6_PMTUDISC_OMIT; > } > > -static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt, > - struct in6_addr *daddr) > +static inline const struct in6_addr *rt6_nexthop(const struct rt6_info *rt, > + const struct in6_addr *daddr) > { > if (rt->rt6i_flags & RTF_GATEWAY) > return &rt->rt6i_gateway; > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c > index 19d27bee285e..1555b0c6f7ec 100644 > --- a/net/bluetooth/6lowpan.c > +++ b/net/bluetooth/6lowpan.c > @@ -160,10 +160,10 @@ static inline struct lowpan_peer *peer_lookup_dst(struct lowpan_btle_dev *dev, > struct in6_addr *daddr, > struct sk_buff *skb) > { > - struct lowpan_peer *peer; > - struct in6_addr *nexthop; > struct rt6_info *rt = (struct rt6_info *)skb_dst(skb); > int count = atomic_read(&dev->peer_count); > + const struct in6_addr *nexthop; > + struct lowpan_peer *peer; I see the added const, but I'm not sure why the declarations were reordered? Here and below. Doesn't matter for code review (doesn't necessitate a v2). > > BT_DBG("peers %d addr %pI6c rt %p", count, daddr, rt); > > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c > index 834475717110..21efcd02f337 100644 > --- a/net/ipv6/ip6_output.c > +++ b/net/ipv6/ip6_output.c > @@ -59,8 +59,8 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * > { > struct dst_entry *dst = skb_dst(skb); > struct net_device *dev = dst->dev; > + const struct in6_addr *nexthop; > struct neighbour *neigh; > - struct in6_addr *nexthop; > int ret; > > if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)) { > diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c > index 241317473114..cdfc33517e85 100644 > --- a/net/netfilter/nf_flow_table_ip.c > +++ b/net/netfilter/nf_flow_table_ip.c > @@ -439,9 +439,9 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb, > struct nf_flowtable *flow_table = priv; > struct flow_offload_tuple tuple = {}; > enum flow_offload_tuple_dir dir; > + const struct in6_addr *nexthop; > struct flow_offload *flow; > struct net_device *outdev; > - struct in6_addr *nexthop; > struct ipv6hdr *ip6h; > struct rt6_info *rt; > > -- > 2.21.0 >
From: Nick Desaulniers <ndesaulniers@google.com> Date: Mon, 24 Jun 2019 09:45:14 -0700 > https://groups.google.com/forum/#!searchin/clang-built-linux/const%7Csort:date/clang-built-linux/umkS84jS9m8/GAVVEgNYBgAJ Inaccessible... This group either doesn't exist, or you don't have permission to access it. If you're sure this group exists, contact the Owner of the group and ask them to give you access. And you mean just changing to 'const' fixes something, how?
On Mon, Jun 24, 2019 at 10:06 AM David Miller <davem@davemloft.net> wrote: > > From: Nick Desaulniers <ndesaulniers@google.com> > Date: Mon, 24 Jun 2019 09:45:14 -0700 > > > https://groups.google.com/forum/#!searchin/clang-built-linux/const%7Csort:date/clang-built-linux/umkS84jS9m8/GAVVEgNYBgAJ > > Inaccessible... > > This group either doesn't exist, or you don't have permission > to access it. If you're sure this group exists, contact the > Owner of the group and ask them to give you access. Sorry, I set up the mailing list not too long ago, seem to have a long tail of permissions related issues. I confirmed that the link was borked in an incognito window. Via https://support.google.com/a/answer/9325317#Visibility I was able to change the obscure setting. I now confirmed the above link works in an incognito window. Thanks for reporting; can you please triple check? > > And you mean just changing to 'const' fixes something, how? See the warning in the above link (assuming now you have access). Assigning a non-const variable the result of a function call that returns const discards the const qualifier.
From: Nick Desaulniers <ndesaulniers@google.com> Date: Mon, 24 Jun 2019 10:17:03 -0700 > On Mon, Jun 24, 2019 at 10:06 AM David Miller <davem@davemloft.net> wrote: >> >> From: Nick Desaulniers <ndesaulniers@google.com> >> Date: Mon, 24 Jun 2019 09:45:14 -0700 >> >> > https://groups.google.com/forum/#!searchin/clang-built-linux/const%7Csort:date/clang-built-linux/umkS84jS9m8/GAVVEgNYBgAJ >> >> Inaccessible... >> >> This group either doesn't exist, or you don't have permission >> to access it. If you're sure this group exists, contact the >> Owner of the group and ask them to give you access. > > Sorry, I set up the mailing list not too long ago, seem to have a long > tail of permissions related issues. I confirmed that the link was > borked in an incognito window. Via > https://support.google.com/a/answer/9325317#Visibility I was able to > change the obscure setting. I now confirmed the above link works in > an incognito window. Thanks for reporting; can you please triple > check? Yep it works now, thanks. >> >> And you mean just changing to 'const' fixes something, how? > > See the warning in the above link (assuming now you have access). > Assigning a non-const variable the result of a function call that > returns const discards the const qualifier. Ok thanks for clarifying. However I was speaking in terms of this fixing a functional bug rather than a loss of const warning.
On Mon, Jun 24, 2019 at 10:22 AM David Miller <davem@davemloft.net> wrote: > > From: Nick Desaulniers <ndesaulniers@google.com> > Date: Mon, 24 Jun 2019 10:17:03 -0700 > > > On Mon, Jun 24, 2019 at 10:06 AM David Miller <davem@davemloft.net> wrote: > >> And you mean just changing to 'const' fixes something, how? > > > > See the warning in the above link (assuming now you have access). > > Assigning a non-const variable the result of a function call that > > returns const discards the const qualifier. > > Ok thanks for clarifying. > > However I was speaking in terms of this fixing a functional bug rather > than a loss of const warning. The author stated that this patch was no functional change. Nicolas, it can be helpful to include compiler warnings in the commit message when sending warning fixes, but it's not a big deal. Thanks for sending the patches.
Le 24/06/2019 à 19:37, Nick Desaulniers a écrit : [snip] > > The author stated that this patch was no functional change. Nicolas, > it can be helpful to include compiler warnings in the commit message > when sending warning fixes, but it's not a big deal. Thanks for > sending the patches. > Yep, but I was not aware of this compilation warning. As explained in the commit log, the goal of this patch was to prepare the next one. Regards, Nicolas
From: Nicolas Dichtel <nicolas.dichtel@6wind.com> Date: Mon, 24 Jun 2019 20:18:37 +0200 > Le 24/06/2019 à 19:37, Nick Desaulniers a écrit : > [snip] >> >> The author stated that this patch was no functional change. Nicolas, >> it can be helpful to include compiler warnings in the commit message >> when sending warning fixes, but it's not a big deal. Thanks for >> sending the patches. >> > Yep, but I was not aware of this compilation warning. As explained in the commit > log, the goal of this patch was to prepare the next one. Yeah, don't worry about it.
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c index 11b9525dff27..311b0cc6eb98 100644 --- a/drivers/net/vrf.c +++ b/drivers/net/vrf.c @@ -350,8 +350,8 @@ static int vrf_finish_output6(struct net *net, struct sock *sk, { struct dst_entry *dst = skb_dst(skb); struct net_device *dev = dst->dev; + const struct in6_addr *nexthop; struct neighbour *neigh; - struct in6_addr *nexthop; int ret; nf_reset(skb); diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h index 4790beaa86e0..ee7405e759ba 100644 --- a/include/net/ip6_route.h +++ b/include/net/ip6_route.h @@ -262,8 +262,8 @@ static inline bool ip6_sk_ignore_df(const struct sock *sk) inet6_sk(sk)->pmtudisc == IPV6_PMTUDISC_OMIT; } -static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt, - struct in6_addr *daddr) +static inline const struct in6_addr *rt6_nexthop(const struct rt6_info *rt, + const struct in6_addr *daddr) { if (rt->rt6i_flags & RTF_GATEWAY) return &rt->rt6i_gateway; diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index 19d27bee285e..1555b0c6f7ec 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -160,10 +160,10 @@ static inline struct lowpan_peer *peer_lookup_dst(struct lowpan_btle_dev *dev, struct in6_addr *daddr, struct sk_buff *skb) { - struct lowpan_peer *peer; - struct in6_addr *nexthop; struct rt6_info *rt = (struct rt6_info *)skb_dst(skb); int count = atomic_read(&dev->peer_count); + const struct in6_addr *nexthop; + struct lowpan_peer *peer; BT_DBG("peers %d addr %pI6c rt %p", count, daddr, rt); diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 834475717110..21efcd02f337 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -59,8 +59,8 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * { struct dst_entry *dst = skb_dst(skb); struct net_device *dev = dst->dev; + const struct in6_addr *nexthop; struct neighbour *neigh; - struct in6_addr *nexthop; int ret; if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)) { diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c index 241317473114..cdfc33517e85 100644 --- a/net/netfilter/nf_flow_table_ip.c +++ b/net/netfilter/nf_flow_table_ip.c @@ -439,9 +439,9 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb, struct nf_flowtable *flow_table = priv; struct flow_offload_tuple tuple = {}; enum flow_offload_tuple_dir dir; + const struct in6_addr *nexthop; struct flow_offload *flow; struct net_device *outdev; - struct in6_addr *nexthop; struct ipv6hdr *ip6h; struct rt6_info *rt;
There is no functional change in this patch, it only prepares the next one. rt6_nexthop() will be used by ip6_dst_lookup_neigh(), which uses const variables. Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- drivers/net/vrf.c | 2 +- include/net/ip6_route.h | 4 ++-- net/bluetooth/6lowpan.c | 4 ++-- net/ipv6/ip6_output.c | 2 +- net/netfilter/nf_flow_table_ip.c | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-)