| Message ID | 1444802314-28830-2-git-send-email-ronen.arad@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 13 Oct 2015 22:58:31 -0700, Ronen Arad wrote: > --- a/include/net/rtnetlink.h > +++ b/include/net/rtnetlink.h > @@ -130,6 +130,8 @@ struct rtnl_af_ops { > const struct nlattr *attr); > int (*set_link_af)(struct net_device *dev, > const struct nlattr *attr); > + size_t (*get_link_af_size_filtered)(const struct net_device *dev, > + u32 ext_filter_mask); > }; There's not much point in having two callbacks doing essentially the same. Why you just don't add the new parameter to the existing get_link_af_size? Looking at what the patch 3 does, the current implementations of the callback can just ignore the new parameter and bridge can remove the current br_get_link_af and rename br_get_link_af_size_filtered to br_get_link_af. Jiri
>-----Original Message----- >From: Jiri Benc [mailto:jbenc@redhat.com] >Sent: Wednesday, October 14, 2015 2:55 AM >To: Arad, Ronen >Cc: netdev@vger.kernel.org >Subject: Re: [PATCH net-next 1/4] rtnetlink: Add get_link_af_size_filtered to >rtnl_af_ops > >On Tue, 13 Oct 2015 22:58:31 -0700, Ronen Arad wrote: >> --- a/include/net/rtnetlink.h >> +++ b/include/net/rtnetlink.h >> @@ -130,6 +130,8 @@ struct rtnl_af_ops { >> const struct nlattr *attr); >> int (*set_link_af)(struct net_device *dev, >> const struct nlattr *attr); >> + size_t (*get_link_af_size_filtered)(const struct >net_device *dev, >> + u32 ext_filter_mask); >> }; > >There's not much point in having two callbacks doing essentially the >same. Why you just don't add the new parameter to the existing >get_link_af_size? Looking at what the patch 3 does, the current >implementations of the callback can just ignore the new parameter and >bridge can remove the current br_get_link_af and rename >br_get_link_af_size_filtered to br_get_link_af. [@Ronen] I agree with you about br_netlink. There was no need for both so I removed br_get_link_af. Changing get_link_af_size signature, however, will require change in other unrelated locations. I wanted to avoid that. The affected location are: net/ipv4/devinet.c:2374: .get_link_af_size = inet_get_link_af_size, net/ipv6/addrconf.c:5868: .get_link_af_size = inet6_get_link_af_size, If there is a consensus that adding ext_filter_mask argument to get_link_af is preferred, I'll submit a v2. > > Jiri > >-- >Jiri Benc -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h index aff6ceb..96df9bb 100644 --- a/include/net/rtnetlink.h +++ b/include/net/rtnetlink.h @@ -130,6 +130,8 @@ struct rtnl_af_ops { const struct nlattr *attr); int (*set_link_af)(struct net_device *dev, const struct nlattr *attr); + size_t (*get_link_af_size_filtered)(const struct net_device *dev, + u32 ext_filter_mask); }; void __rtnl_af_unregister(struct rtnl_af_ops *ops);
get_link_af_size_filtered() - a filtering-mask aware alternative function is added to struct rtnl_af_ops in order to allow for "rightsizing" the IFLA_AF_SPEC calculation in if_nlmsg_size(). This significantly reduces the message size when at least one netdev has large number of VLANs. Signed-off-by: Ronen Arad <ronen.arad@intel.com> --- include/net/rtnetlink.h | 2 ++ 1 file changed, 2 insertions(+)