| Message ID | 7ba2ca17347249b980731e7a76ba3e24a9e37720.1596468610.git.lucien.xin@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: fix a mcast issue for tipc udp media | expand |
On 8/3/20 11:34 PM, Xin Long wrote: > This is to add an ip_dev_find like function for ipv6, used to find > the dev by saddr. > > It will be used by TIPC protocol. So also export it. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> Acked-by: Ying Xue <ying.xue@windriver.com> > --- > include/net/addrconf.h | 2 ++ > net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+) > > diff --git a/include/net/addrconf.h b/include/net/addrconf.h > index 8418b7d..ba3f6c15 100644 > --- a/include/net/addrconf.h > +++ b/include/net/addrconf.h > @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, > > int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); > > +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); > + > struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, > const struct in6_addr *addr, > struct net_device *dev, int strict); > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index 840bfdb..857d6f9 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -1983,6 +1983,45 @@ int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev) > } > EXPORT_SYMBOL(ipv6_chk_prefix); > > +/** > + * ipv6_dev_find - find the first device with a given source address. > + * @net: the net namespace > + * @addr: the source address > + * > + * The caller should be protected by RCU, or RTNL. > + */ > +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr) > +{ > + unsigned int hash = inet6_addr_hash(net, addr); > + struct inet6_ifaddr *ifp, *result = NULL; > + struct net_device *dev = NULL; > + > + rcu_read_lock(); > + hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) { > + if (net_eq(dev_net(ifp->idev->dev), net) && > + ipv6_addr_equal(&ifp->addr, addr)) { > + result = ifp; > + break; > + } > + } > + > + if (!result) { > + struct rt6_info *rt; > + > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > + if (rt) { > + dev = rt->dst.dev; > + ip6_rt_put(rt); > + } > + } else { > + dev = result->idev->dev; > + } > + rcu_read_unlock(); > + > + return dev; > +} > +EXPORT_SYMBOL(ipv6_dev_find); > + > struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *addr, > struct net_device *dev, int strict) > { >
On 8/5/20 5:03 AM, Ying Xue wrote: >> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c >> index 840bfdb..857d6f9 100644 >> --- a/net/ipv6/addrconf.c >> +++ b/net/ipv6/addrconf.c >> @@ -1983,6 +1983,45 @@ int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev) >> } >> EXPORT_SYMBOL(ipv6_chk_prefix); >> >> +/** >> + * ipv6_dev_find - find the first device with a given source address. >> + * @net: the net namespace >> + * @addr: the source address >> + * >> + * The caller should be protected by RCU, or RTNL. >> + */ >> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr) >> +{ >> + unsigned int hash = inet6_addr_hash(net, addr); >> + struct inet6_ifaddr *ifp, *result = NULL; >> + struct net_device *dev = NULL; >> + >> + rcu_read_lock(); >> + hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) { >> + if (net_eq(dev_net(ifp->idev->dev), net) && >> + ipv6_addr_equal(&ifp->addr, addr)) { >> + result = ifp; >> + break; >> + } >> + } >> + >> + if (!result) { >> + struct rt6_info *rt; >> + >> + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); why is this needed? you have already walked the interfaces in the namespace and not found the address. Doing a route lookup is beyond the stated scope of this function. >> + if (rt) { >> + dev = rt->dst.dev; >> + ip6_rt_put(rt); >> + } >> + } else { >> + dev = result->idev->dev; >> + } >> + rcu_read_unlock(); >> + >> + return dev; >> +} >> +EXPORT_SYMBOL(ipv6_dev_find); >> + >> struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *addr, >> struct net_device *dev, int strict) >> { >>
Hi, 2020年8月4日(火) 0:35 Xin Long <lucien.xin@gmail.com>: > > This is to add an ip_dev_find like function for ipv6, used to find > the dev by saddr. > > It will be used by TIPC protocol. So also export it. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > include/net/addrconf.h | 2 ++ > net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+) > > diff --git a/include/net/addrconf.h b/include/net/addrconf.h > index 8418b7d..ba3f6c15 100644 > --- a/include/net/addrconf.h > +++ b/include/net/addrconf.h > @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, > > int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); > > +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); > + How do we handle link-local addresses? --yoshfuji > struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, > const struct in6_addr *addr, > struct net_device *dev, int strict); > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > index 840bfdb..857d6f9 100644 > --- a/net/ipv6/addrconf.c > +++ b/net/ipv6/addrconf.c > @@ -1983,6 +1983,45 @@ int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev) > } > EXPORT_SYMBOL(ipv6_chk_prefix); > > +/** > + * ipv6_dev_find - find the first device with a given source address. > + * @net: the net namespace > + * @addr: the source address > + * > + * The caller should be protected by RCU, or RTNL. > + */ > +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr) > +{ > + unsigned int hash = inet6_addr_hash(net, addr); > + struct inet6_ifaddr *ifp, *result = NULL; > + struct net_device *dev = NULL; > + > + rcu_read_lock(); > + hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) { > + if (net_eq(dev_net(ifp->idev->dev), net) && > + ipv6_addr_equal(&ifp->addr, addr)) { > + result = ifp; > + break; > + } > + } > + > + if (!result) { > + struct rt6_info *rt; > + > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > + if (rt) { > + dev = rt->dst.dev; > + ip6_rt_put(rt); > + } > + } else { > + dev = result->idev->dev; > + } > + rcu_read_unlock(); > + > + return dev; > +} > +EXPORT_SYMBOL(ipv6_dev_find); > + > struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *addr, > struct net_device *dev, int strict) > { > -- > 2.1.0 >
On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com> wrote: > > Hi, > > 2020年8月4日(火) 0:35 Xin Long <lucien.xin@gmail.com>: > > > > This is to add an ip_dev_find like function for ipv6, used to find > > the dev by saddr. > > > > It will be used by TIPC protocol. So also export it. > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > > --- > > include/net/addrconf.h | 2 ++ > > net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 41 insertions(+) > > > > diff --git a/include/net/addrconf.h b/include/net/addrconf.h > > index 8418b7d..ba3f6c15 100644 > > --- a/include/net/addrconf.h > > +++ b/include/net/addrconf.h > > @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, > > > > int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); > > > > +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); > > + > > How do we handle link-local addresses? This is what "if (!result)" branch meant to do: + if (!result) { + struct rt6_info *rt; + + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); + if (rt) { + dev = rt->dst.dev; + ip6_rt_put(rt); + } + } else { + dev = result->idev->dev; + } Thanks. > > --yoshfuji > > > struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, > > const struct in6_addr *addr, > > struct net_device *dev, int strict); > > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c > > index 840bfdb..857d6f9 100644 > > --- a/net/ipv6/addrconf.c > > +++ b/net/ipv6/addrconf.c > > @@ -1983,6 +1983,45 @@ int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev) > > } > > EXPORT_SYMBOL(ipv6_chk_prefix); > > > > +/** > > + * ipv6_dev_find - find the first device with a given source address. > > + * @net: the net namespace > > + * @addr: the source address > > + * > > + * The caller should be protected by RCU, or RTNL. > > + */ > > +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr) > > +{ > > + unsigned int hash = inet6_addr_hash(net, addr); > > + struct inet6_ifaddr *ifp, *result = NULL; > > + struct net_device *dev = NULL; > > + > > + rcu_read_lock(); > > + hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) { > > + if (net_eq(dev_net(ifp->idev->dev), net) && > > + ipv6_addr_equal(&ifp->addr, addr)) { > > + result = ifp; > > + break; > > + } > > + } > > + > > + if (!result) { > > + struct rt6_info *rt; > > + > > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > > + if (rt) { > > + dev = rt->dst.dev; > > + ip6_rt_put(rt); > > + } > > + } else { > > + dev = result->idev->dev; > > + } > > + rcu_read_unlock(); > > + > > + return dev; > > +} > > +EXPORT_SYMBOL(ipv6_dev_find); > > + > > struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *addr, > > struct net_device *dev, int strict) > > { > > -- > > 2.1.0 > >
On 8/6/20 2:55 AM, Xin Long wrote: > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji > <hideaki.yoshifuji@miraclelinux.com> wrote: >> >> Hi, >> >> 2020年8月4日(火) 0:35 Xin Long <lucien.xin@gmail.com>: >>> >>> This is to add an ip_dev_find like function for ipv6, used to find >>> the dev by saddr. >>> >>> It will be used by TIPC protocol. So also export it. >>> >>> Signed-off-by: Xin Long <lucien.xin@gmail.com> >>> --- >>> include/net/addrconf.h | 2 ++ >>> net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 41 insertions(+) >>> >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h >>> index 8418b7d..ba3f6c15 100644 >>> --- a/include/net/addrconf.h >>> +++ b/include/net/addrconf.h >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, >>> >>> int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); >>> >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); >>> + >> >> How do we handle link-local addresses? > This is what "if (!result)" branch meant to do: > > + if (!result) { > + struct rt6_info *rt; > + > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > + if (rt) { > + dev = rt->dst.dev; > + ip6_rt_put(rt); > + } > + } else { > + dev = result->idev->dev; > + } > the stated purpose of this function is to find the netdevice to which an address is attached. A route lookup should not be needed. Walking the address hash list finds the address and hence the netdev or it does not.
On Thu, Aug 6, 2020 at 10:03 PM David Ahern <dsahern@gmail.com> wrote: > > On 8/6/20 2:55 AM, Xin Long wrote: > > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji > > <hideaki.yoshifuji@miraclelinux.com> wrote: > >> > >> Hi, > >> > >> 2020年8月4日(火) 0:35 Xin Long <lucien.xin@gmail.com>: > >>> > >>> This is to add an ip_dev_find like function for ipv6, used to find > >>> the dev by saddr. > >>> > >>> It will be used by TIPC protocol. So also export it. > >>> > >>> Signed-off-by: Xin Long <lucien.xin@gmail.com> > >>> --- > >>> include/net/addrconf.h | 2 ++ > >>> net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ > >>> 2 files changed, 41 insertions(+) > >>> > >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h > >>> index 8418b7d..ba3f6c15 100644 > >>> --- a/include/net/addrconf.h > >>> +++ b/include/net/addrconf.h > >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, > >>> > >>> int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); > >>> > >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); > >>> + > >> > >> How do we handle link-local addresses? > > This is what "if (!result)" branch meant to do: > > > > + if (!result) { > > + struct rt6_info *rt; > > + > > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > > + if (rt) { > > + dev = rt->dst.dev; > > + ip6_rt_put(rt); > > + } > > + } else { > > + dev = result->idev->dev; > > + } > > > > the stated purpose of this function is to find the netdevice to which an > address is attached. A route lookup should not be needed. Walking the > address hash list finds the address and hence the netdev or it does not. Hi, David, Sorry. it does. I misunderstood the code in __ip_dev_find(). I will delete the rt6_lookup() part from ipv6_dev_find(). Also for the compatibility, tipc part should change to: @@ -741,10 +741,8 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b, struct net_device *dev; dev = ipv6_dev_find(net, &local.ipv6); if (!dev) ub->ifindex = dev->ifindex; as when dev is not found from the hash list, it should fall back to the old tipc code. Ying, what do you think?
Hi, 2020年8月6日(木) 23:03 David Ahern <dsahern@gmail.com>: > > On 8/6/20 2:55 AM, Xin Long wrote: > > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji > > <hideaki.yoshifuji@miraclelinux.com> wrote: > >> > >> Hi, > >> > >> 2020年8月4日(火) 0:35 Xin Long <lucien.xin@gmail.com>: > >>> > >>> This is to add an ip_dev_find like function for ipv6, used to find > >>> the dev by saddr. > >>> > >>> It will be used by TIPC protocol. So also export it. > >>> > >>> Signed-off-by: Xin Long <lucien.xin@gmail.com> > >>> --- > >>> include/net/addrconf.h | 2 ++ > >>> net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ > >>> 2 files changed, 41 insertions(+) > >>> > >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h > >>> index 8418b7d..ba3f6c15 100644 > >>> --- a/include/net/addrconf.h > >>> +++ b/include/net/addrconf.h > >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, > >>> > >>> int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); > >>> > >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); > >>> + > >> > >> How do we handle link-local addresses? > > This is what "if (!result)" branch meant to do: > > > > + if (!result) { > > + struct rt6_info *rt; > > + > > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > > + if (rt) { > > + dev = rt->dst.dev; > > + ip6_rt_put(rt); > > + } > > + } else { > > + dev = result->idev->dev; > > + } > > > > the stated purpose of this function is to find the netdevice to which an > address is attached. A route lookup should not be needed. Walking the > address hash list finds the address and hence the netdev or it does not. > > User supplied scope id which should be set for link-local addresses in TIPC_NLA_UDP_LOCAL attribute must be honored when we check the address. ipv6_chk_addr() can check if the address and supplied ifindex is a valid local address. Or introduce an extra ifindex argument to ipv6_dev_find(). --yoshfuji
On Fri, Aug 7, 2020 at 5:26 PM Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com> wrote: > > Hi, > > 2020年8月6日(木) 23:03 David Ahern <dsahern@gmail.com>: > > > > On 8/6/20 2:55 AM, Xin Long wrote: > > > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji > > > <hideaki.yoshifuji@miraclelinux.com> wrote: > > >> > > >> Hi, > > >> > > >> 2020年8月4日(火) 0:35 Xin Long <lucien.xin@gmail.com>: > > >>> > > >>> This is to add an ip_dev_find like function for ipv6, used to find > > >>> the dev by saddr. > > >>> > > >>> It will be used by TIPC protocol. So also export it. > > >>> > > >>> Signed-off-by: Xin Long <lucien.xin@gmail.com> > > >>> --- > > >>> include/net/addrconf.h | 2 ++ > > >>> net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ > > >>> 2 files changed, 41 insertions(+) > > >>> > > >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h > > >>> index 8418b7d..ba3f6c15 100644 > > >>> --- a/include/net/addrconf.h > > >>> +++ b/include/net/addrconf.h > > >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, > > >>> > > >>> int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); > > >>> > > >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); > > >>> + > > >> > > >> How do we handle link-local addresses? > > > This is what "if (!result)" branch meant to do: > > > > > > + if (!result) { > > > + struct rt6_info *rt; > > > + > > > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > > > + if (rt) { > > > + dev = rt->dst.dev; > > > + ip6_rt_put(rt); > > > + } > > > + } else { > > > + dev = result->idev->dev; > > > + } > > > > > > > the stated purpose of this function is to find the netdevice to which an > > address is attached. A route lookup should not be needed. Walking the > > address hash list finds the address and hence the netdev or it does not. > > > > > > User supplied scope id which should be set for link-local addresses > in TIPC_NLA_UDP_LOCAL attribute must be honored when we > check the address. Hi, Hideaki san, Sorry for not understanding your comment earlier. The bad thing is tipc in iproute2 doesn't seem able to set scope_id. I saw many places in kernel doing this check: if (__ipv6_addr_needs_scope_id(atype) && !ip6->sin6_scope_id) { return -EINVAL; } Can I ask why scope id is needed for link-local addresses? and is that for link-local addresses only? > > ipv6_chk_addr() can check if the address and supplied ifindex is a valid > local address. Or introduce an extra ifindex argument to ipv6_dev_find(). Yeah, but if scope id means ifindex for link-local addresses, ipv6_dev_find() would be more like a function to validate the address with right scope id. Thanks for your reviewing.
Hi, 2020年8月9日(日) 19:52 Xin Long <lucien.xin@gmail.com>: > > On Fri, Aug 7, 2020 at 5:26 PM Hideaki Yoshifuji > <hideaki.yoshifuji@miraclelinux.com> wrote: > > > > Hi, > > > > 2020年8月6日(木) 23:03 David Ahern <dsahern@gmail.com>: > > > > > > On 8/6/20 2:55 AM, Xin Long wrote: > > > > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji > > > > <hideaki.yoshifuji@miraclelinux.com> wrote: > > > >> > > > >> Hi, > > > >> > > > >> 2020年8月4日(火) 0:35 Xin Long <lucien.xin@gmail.com>: > > > >>> > > > >>> This is to add an ip_dev_find like function for ipv6, used to find > > > >>> the dev by saddr. > > > >>> > > > >>> It will be used by TIPC protocol. So also export it. > > > >>> > > > >>> Signed-off-by: Xin Long <lucien.xin@gmail.com> > > > >>> --- > > > >>> include/net/addrconf.h | 2 ++ > > > >>> net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ > > > >>> 2 files changed, 41 insertions(+) > > > >>> > > > >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h > > > >>> index 8418b7d..ba3f6c15 100644 > > > >>> --- a/include/net/addrconf.h > > > >>> +++ b/include/net/addrconf.h > > > >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, > > > >>> > > > >>> int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); > > > >>> > > > >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); > > > >>> + > > > >> > > > >> How do we handle link-local addresses? > > > > This is what "if (!result)" branch meant to do: > > > > > > > > + if (!result) { > > > > + struct rt6_info *rt; > > > > + > > > > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > > > > + if (rt) { > > > > + dev = rt->dst.dev; > > > > + ip6_rt_put(rt); > > > > + } > > > > + } else { > > > > + dev = result->idev->dev; > > > > + } > > > > > > > > > > the stated purpose of this function is to find the netdevice to which an > > > address is attached. A route lookup should not be needed. Walking the > > > address hash list finds the address and hence the netdev or it does not. > > > > > > > > > > User supplied scope id which should be set for link-local addresses > > in TIPC_NLA_UDP_LOCAL attribute must be honored when we > > check the address. > Hi, Hideaki san, > > Sorry for not understanding your comment earlier. > > The bad thing is tipc in iproute2 doesn't seem able to set scope_id. I looked into the iproute2 code quickly and I think it should; it uses getaddrinfo(3) and it will fill if you say "fe80::1%eth0" or something like that.... OR, fix the bug. > I saw many places in kernel doing this check: > > if (__ipv6_addr_needs_scope_id(atype) && > !ip6->sin6_scope_id) { return -EINVAL; } > > Can I ask why scope id is needed for link-local addresses? > and is that for link-local addresses only? Because we distinguish link-local scope addresses on different interfaces. On the other hand, we do not distinguish global scope addresses on different interfaces. > > > > > ipv6_chk_addr() can check if the address and supplied ifindex is a valid > > local address. Or introduce an extra ifindex argument to ipv6_dev_find(). > Yeah, but if scope id means ifindex for link-local addresses, ipv6_dev_find() > would be more like a function to validate the address with right scope id. > I think we should find a net_device with a specific "valid" (non-tentative) address here, and your initial implementation is not enough because it does not reject tentative addresses. I'd recommend using generic ipv6_chk_addr() inside. > Thanks for your reviewing.
On Tue, Aug 11, 2020 at 10:26 AM Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com> wrote: > > Hi, > > 2020年8月9日(日) 19:52 Xin Long <lucien.xin@gmail.com>: > > > > On Fri, Aug 7, 2020 at 5:26 PM Hideaki Yoshifuji > > <hideaki.yoshifuji@miraclelinux.com> wrote: > > > > > > Hi, > > > > > > 2020年8月6日(木) 23:03 David Ahern <dsahern@gmail.com>: > > > > > > > > On 8/6/20 2:55 AM, Xin Long wrote: > > > > > On Thu, Aug 6, 2020 at 10:50 AM Hideaki Yoshifuji > > > > > <hideaki.yoshifuji@miraclelinux.com> wrote: > > > > >> > > > > >> Hi, > > > > >> > > > > >> 2020年8月4日(火) 0:35 Xin Long <lucien.xin@gmail.com>: > > > > >>> > > > > >>> This is to add an ip_dev_find like function for ipv6, used to find > > > > >>> the dev by saddr. > > > > >>> > > > > >>> It will be used by TIPC protocol. So also export it. > > > > >>> > > > > >>> Signed-off-by: Xin Long <lucien.xin@gmail.com> > > > > >>> --- > > > > >>> include/net/addrconf.h | 2 ++ > > > > >>> net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ > > > > >>> 2 files changed, 41 insertions(+) > > > > >>> > > > > >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h > > > > >>> index 8418b7d..ba3f6c15 100644 > > > > >>> --- a/include/net/addrconf.h > > > > >>> +++ b/include/net/addrconf.h > > > > >>> @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, > > > > >>> > > > > >>> int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); > > > > >>> > > > > >>> +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); > > > > >>> + > > > > >> > > > > >> How do we handle link-local addresses? > > > > > This is what "if (!result)" branch meant to do: > > > > > > > > > > + if (!result) { > > > > > + struct rt6_info *rt; > > > > > + > > > > > + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); > > > > > + if (rt) { > > > > > + dev = rt->dst.dev; > > > > > + ip6_rt_put(rt); > > > > > + } > > > > > + } else { > > > > > + dev = result->idev->dev; > > > > > + } > > > > > > > > > > > > > the stated purpose of this function is to find the netdevice to which an > > > > address is attached. A route lookup should not be needed. Walking the > > > > address hash list finds the address and hence the netdev or it does not. > > > > > > > > > > > > > > User supplied scope id which should be set for link-local addresses > > > in TIPC_NLA_UDP_LOCAL attribute must be honored when we > > > check the address. > > Hi, Hideaki san, > > > > Sorry for not understanding your comment earlier. > > > > The bad thing is tipc in iproute2 doesn't seem able to set scope_id. > > I looked into the iproute2 code quickly and I think it should; it uses > getaddrinfo(3) and it will fill if you say "fe80::1%eth0" or something > like that.... OR, fix the bug. right, thanks. > > > I saw many places in kernel doing this check: > > > > if (__ipv6_addr_needs_scope_id(atype) && > > !ip6->sin6_scope_id) { return -EINVAL; } > > > > Can I ask why scope id is needed for link-local addresses? > > and is that for link-local addresses only? > > Because we distinguish link-local scope addresses on different interfaces. > On the other hand, we do not distinguish global scope addresses on > different interfaces. okay. > > > > > > > > > ipv6_chk_addr() can check if the address and supplied ifindex is a valid > > > local address. Or introduce an extra ifindex argument to ipv6_dev_find(). > > Yeah, but if scope id means ifindex for link-local addresses, ipv6_dev_find() > > would be more like a function to validate the address with right scope id. > > > > I think we should find a net_device with a specific "valid" (non-tentative) > address here, and your initial implementation is not enough because it does > not reject tentative addresses. I'd recommend using generic ipv6_chk_addr() > inside. ipv6_chk_addr() is calling ipv6_chk_addr_and_flags(), which traverses the addr hash list again. So I'm thinking to reuse the code of ipv6_chk_addr_and_flags(), and do: +static struct net_device * +__ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr, + const struct net_device *dev, bool skip_dev_check, + int strict, u32 banned_flags) { unsigned int hash = inet6_addr_hash(net, addr); const struct net_device *l3mdev; @@ -1926,12 +1918,29 @@ int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr, (!dev || ifp->idev->dev == dev || !(ifp->scope&(IFA_LINK|IFA_HOST) || strict))) { rcu_read_unlock(); - return 1; + return ifp->idev->dev; } } rcu_read_unlock(); - return 0; + return NULL; +} and change these functions to : int ipv6_chk_addr_and_flags(struct net *net, const struct in6_addr *addr, const struct net_device *dev, bool skip_dev_check, int strict, u32 banned_flags) { return __ipv6_chk_addr_and_flags(net, addr, dev, skip_dev_check, strict, banned_flags) ? 1 : 0; } EXPORT_SYMBOL(ipv6_chk_addr_and_flags); struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr, struct net_device *dev) { return __ipv6_chk_addr_and_flags(net, addr, NULL, 0, 1, IFA_F_TENTATIVE); } EXPORT_SYMBOL(ipv6_dev_find); what do you think?
diff --git a/include/net/addrconf.h b/include/net/addrconf.h index 8418b7d..ba3f6c15 100644 --- a/include/net/addrconf.h +++ b/include/net/addrconf.h @@ -97,6 +97,8 @@ bool ipv6_chk_custom_prefix(const struct in6_addr *addr, int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev); +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr); + struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *addr, struct net_device *dev, int strict); diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 840bfdb..857d6f9 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -1983,6 +1983,45 @@ int ipv6_chk_prefix(const struct in6_addr *addr, struct net_device *dev) } EXPORT_SYMBOL(ipv6_chk_prefix); +/** + * ipv6_dev_find - find the first device with a given source address. + * @net: the net namespace + * @addr: the source address + * + * The caller should be protected by RCU, or RTNL. + */ +struct net_device *ipv6_dev_find(struct net *net, const struct in6_addr *addr) +{ + unsigned int hash = inet6_addr_hash(net, addr); + struct inet6_ifaddr *ifp, *result = NULL; + struct net_device *dev = NULL; + + rcu_read_lock(); + hlist_for_each_entry_rcu(ifp, &inet6_addr_lst[hash], addr_lst) { + if (net_eq(dev_net(ifp->idev->dev), net) && + ipv6_addr_equal(&ifp->addr, addr)) { + result = ifp; + break; + } + } + + if (!result) { + struct rt6_info *rt; + + rt = rt6_lookup(net, addr, NULL, 0, NULL, 0); + if (rt) { + dev = rt->dst.dev; + ip6_rt_put(rt); + } + } else { + dev = result->idev->dev; + } + rcu_read_unlock(); + + return dev; +} +EXPORT_SYMBOL(ipv6_dev_find); + struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *addr, struct net_device *dev, int strict) {
This is to add an ip_dev_find like function for ipv6, used to find the dev by saddr. It will be used by TIPC protocol. So also export it. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- include/net/addrconf.h | 2 ++ net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+)