| Message ID | 20240730003010.156977-1-maze@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 246ef40670b71fef0c3e2cd11404279bc6d6468e |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] ipv6: eliminate ndisc_ops_is_useropt() | expand |
On 7/30/24 02:30, Maciej Żenczykowski wrote: > as it doesn't seem to offer anything of value. > > There's only 1 trivial user: > int lowpan_ndisc_is_useropt(u8 nd_opt_type) { > return nd_opt_type == ND_OPT_6CO; > } > > but there's no harm to always treating that as > a useropt... AFAICS there will be an user-visible difference, as the user-space could start receiving "unexpected" notification for such opt even when the kernel is built with CONFIG_6LOWPAN=n. The user-space should ignore unknown/unexpected notification,so it should be safe, but I'm a bit unsettled by the many 'should' ;) Cheers, Paolo
On Thu, Aug 1, 2024 at 6:54 AM Paolo Abeni <pabeni@redhat.com> wrote: > > On 7/30/24 02:30, Maciej Żenczykowski wrote: > > as it doesn't seem to offer anything of value. > > > > There's only 1 trivial user: > > int lowpan_ndisc_is_useropt(u8 nd_opt_type) { > > return nd_opt_type == ND_OPT_6CO; > > } > > > > but there's no harm to always treating that as > > a useropt... > > AFAICS there will be an user-visible difference, as the user-space could > start receiving "unexpected" notification for such opt even when the > kernel is built with CONFIG_6LOWPAN=n. > > The user-space should ignore unknown/unexpected notification,so it > should be safe, but I'm a bit unsettled by the many 'should' ;) We've added new options to this list in the past. Even going so far as shipping those changes via LTS... So IMHO this doesn't seem like a real concern. Also the option is very unlikely to be present in normal non 6lowpan environments. > Cheers, > > Paolo >
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 29 Jul 2024 17:30:10 -0700 you wrote: > as it doesn't seem to offer anything of value. > > There's only 1 trivial user: > int lowpan_ndisc_is_useropt(u8 nd_opt_type) { > return nd_opt_type == ND_OPT_6CO; > } > > [...] Here is the summary with links: - [net-next] ipv6: eliminate ndisc_ops_is_useropt() https://git.kernel.org/netdev/net-next/c/246ef40670b7 You are awesome, thank you!
diff --git a/include/net/ndisc.h b/include/net/ndisc.h index 7a533d5b1d59..3c88d5bc5eed 100644 --- a/include/net/ndisc.h +++ b/include/net/ndisc.h @@ -147,11 +147,6 @@ void __ndisc_fill_addr_option(struct sk_buff *skb, int type, const void *data, * The following hooks can be defined; unless noted otherwise, they are * optional and can be filled with a null pointer. * - * int (*is_useropt)(u8 nd_opt_type): - * This function is called when IPv6 decide RA userspace options. if - * this function returns 1 then the option given by nd_opt_type will - * be handled as userspace option additional to the IPv6 options. - * * int (*parse_options)(const struct net_device *dev, * struct nd_opt_hdr *nd_opt, * struct ndisc_options *ndopts): @@ -200,7 +195,6 @@ void __ndisc_fill_addr_option(struct sk_buff *skb, int type, const void *data, * addresses. E.g. 802.15.4 6LoWPAN. */ struct ndisc_ops { - int (*is_useropt)(u8 nd_opt_type); int (*parse_options)(const struct net_device *dev, struct nd_opt_hdr *nd_opt, struct ndisc_options *ndopts); @@ -224,15 +218,6 @@ struct ndisc_ops { }; #if IS_ENABLED(CONFIG_IPV6) -static inline int ndisc_ops_is_useropt(const struct net_device *dev, - u8 nd_opt_type) -{ - if (dev->ndisc_ops && dev->ndisc_ops->is_useropt) - return dev->ndisc_ops->is_useropt(nd_opt_type); - else - return 0; -} - static inline int ndisc_ops_parse_options(const struct net_device *dev, struct nd_opt_hdr *nd_opt, struct ndisc_options *ndopts) diff --git a/net/6lowpan/ndisc.c b/net/6lowpan/ndisc.c index 16be8f8b2f8c..c40b98f7743c 100644 --- a/net/6lowpan/ndisc.c +++ b/net/6lowpan/ndisc.c @@ -11,11 +11,6 @@ #include "6lowpan_i.h" -static int lowpan_ndisc_is_useropt(u8 nd_opt_type) -{ - return nd_opt_type == ND_OPT_6CO; -} - #if IS_ENABLED(CONFIG_IEEE802154_6LOWPAN) #define NDISC_802154_SHORT_ADDR_LENGTH 1 static int lowpan_ndisc_parse_802154_options(const struct net_device *dev, @@ -222,7 +217,6 @@ static void lowpan_ndisc_prefix_rcv_add_addr(struct net *net, #endif const struct ndisc_ops lowpan_ndisc_ops = { - .is_useropt = lowpan_ndisc_is_useropt, #if IS_ENABLED(CONFIG_IEEE802154_6LOWPAN) .parse_options = lowpan_ndisc_parse_options, .update = lowpan_ndisc_update, diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index b8eec1b6cc2c..1e42e40fb379 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -200,9 +200,9 @@ static inline int ndisc_is_useropt(const struct net_device *dev, return opt->nd_opt_type == ND_OPT_PREFIX_INFO || opt->nd_opt_type == ND_OPT_RDNSS || opt->nd_opt_type == ND_OPT_DNSSL || + opt->nd_opt_type == ND_OPT_6CO || opt->nd_opt_type == ND_OPT_CAPTIVE_PORTAL || - opt->nd_opt_type == ND_OPT_PREF64 || - ndisc_ops_is_useropt(dev, opt->nd_opt_type); + opt->nd_opt_type == ND_OPT_PREF64; } static struct nd_opt_hdr *ndisc_next_useropt(const struct net_device *dev,
as it doesn't seem to offer anything of value. There's only 1 trivial user: int lowpan_ndisc_is_useropt(u8 nd_opt_type) { return nd_opt_type == ND_OPT_6CO; } but there's no harm to always treating that as a useropt... Cc: David Ahern <dsahern@kernel.org> Cc: YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> Cc: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Maciej Żenczykowski <maze@google.com> --- include/net/ndisc.h | 15 --------------- net/6lowpan/ndisc.c | 6 ------ net/ipv6/ndisc.c | 4 ++-- 3 files changed, 2 insertions(+), 23 deletions(-)