Re: [ovs-dev] [PATCH v4] netlink: ignore IFLA_WIRELESS events
On Thu, 1 Apr 2021 at 21:17, Ilya Maximets wrote:
> On 3/4/21 4:32 PM, Michal Kazior wrote:
[...]
> It's unclear why we need to check inside these functions.
> I mean, there is only one place where these functions called
> and there is no any useful work done there beside calling them.
> I think, it's better to just check right after receiving
> the change in a same way as in netdev_linux_update_via_netlink().
>
> Something like this:
>
[...]
> -if (rtnetlink_parse(&buf, &change)) {
> +if (rtnetlink_parse(&buf, &change) && !change->irrelevant) {
[...]
>
> What do you think?
> If it looks good to you, I can squash above diff with your patch and
> apply to master.
No particular reason why I did it like that. But you're right. I don't
mind if you change it. Thanks!
Michał
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] netlink: ignore IFLA_WIRELESS events
On Mon, Mar 1, 2021 at 8:40 PM Ilya Maximets wrote: > > On 1/14/21 10:09 AM, Michal Kazior wrote: > > From: Michal Kazior [...] > Hi, Michal. Thanks for working on this! > The idea seems reasonable to me. Some comments for the implementation > inline. [...] > > diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c > > index 125802925..316524c0f 100644 > > --- a/lib/rtnetlink.c > > +++ b/lib/rtnetlink.c > > @@ -82,7 +82,7 @@ rtnetlink_parse_link_info(const struct nlattr *nla, > > /* Parses a rtnetlink message 'buf' into 'change'. If 'buf' is > > unparseable, > > * leaves 'change' untouched and returns false. Otherwise, populates > > 'change' > > * and returns true. */ > > Comment above is no longer correct with this change. > In general, changing the semantics of a 'parse' method looks > a bit tricky. Maybe we can do this a bit differently? > I'd suggest to keep the 'rtnetlink_parse' almost as is, but > add some extra flag to 'struct rtnetlink_change', that will > be set for change events that are not interesting for OVS. > > Something like 'bool irrelevant;' 'rtnetlink_parse' will set > it to 'true' for wireless events with a fair comment that > "wireless events never really change interface state". > So, 'nln_run' will only report events if this flag is not > set and netdev-linux will check for: > if (rtnetlink_parse(&buf, &change) && !change->irrelevant) > > What do you think? It does sound reasonable. I think I did consider that back when I was cooking this but apparently I discarded the idea for some reason that I no longer remember. I'll give it a try. Michał ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] netlink: ignore IFLA_WIRELESS events
On Tue, 9 Feb 2021 at 00:58, Gregory Rose wrote: > On 1/14/2021 1:09 AM, Michal Kazior wrote: > > From: Michal Kazior [...] > > Hi Michal, > > The patch looks fine to me. Applies cleanly to master > and did not cause any regressions in 'make check'. > > I'm curious since I don't ever use OVS with wireless ports > if there is somewhere this issue has been reported. If > so then we should add it to the commit message. Hi Greg, I'm unaware of any reports of this sort and I wouldn't be surprised to be the first one. Given my experience - and the fact this has been sitting on the mailing list for almost 2 months with no comments now - I imagine not many people tried mixing OVS with wext drivers, especially vendor drivers. Michał ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
