| Message ID | cf82f80f72174f0892c1f9f4e1a8c270@joynext.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] nl80211: Pass "global" events to all interfaces | expand |
Hi, On Thu, 2024-08-29 at 08:01 +0000, Cermak Dominik wrote: > Extending commit f136837202393a7e1f3182e9efdbf1aaa0c1a5c2. Usually the format used is commit <12 sha1 characters> ("message") which would at least give the first line/subject of the commit message. In this case, a little bit more context is probably a good idea, especially if you reference it later using "still". > We got connection failures because of outdated channel information. > That's because the NL80211_CMD_REG_CHANGE event is important for all > interfaces, but the early termination still kicks in because it is not > directed to a specific wiphy. > Therefore from three interfaces, only one got the updated channel list. > > Fix this by changing the early termination logic to only apply to events > directed either to a specific interface index for wdev. The logic looks good to me. Benjamin > Signed-off-by: Dominik Cermak <dominik.cermak@joynext.com> > --- > src/drivers/driver_nl80211_event.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/drivers/driver_nl80211_event.c > b/src/drivers/driver_nl80211_event.c > index 903207067..8e79b5848 100644 > --- a/src/drivers/driver_nl80211_event.c > +++ b/src/drivers/driver_nl80211_event.c > @@ -4257,7 +4257,13 @@ int process_global_event(struct nl_msg *msg, > void *arg) > wdev_id == bss->wdev_id)) { > processed = true; > do_process_drv_event(bss, gnlh->cmd, > tb); > - if (!wiphy_idx_set) > + /* There are two types of events > that may need to be > + * delivered to multiple interfaces: > + * 1. Events for a wiphy, as it can > have multiple interfaces. > + * 2. "Global" events, like > NL80211_CMD_REG_CHANGE. > + * Or in other words, terminate > early only if it is > + * directed to a specific interface > or wdev. */ > + if (ifidx != -1 || wdev_id_set) > return NL_SKIP; > /* The driver instance could have > been removed, > * e.g., due to > NL80211_CMD_RADAR_DETECT event,
Hi Benjamin, thanks for the feedback! >On Thu, 2024-08-29 at 08:01 +0000, Cermak Dominik wrote: >> Extending commit f136837202393a7e1f3182e9efdbf1aaa0c1a5c2. > >Usually the format used is > commit <12 sha1 characters> ("message") >which would at least give the first line/subject of the commit message. > >In this case, a little bit more context is probably a good idea, >especially if you reference it later using "still". Here is my try at improving the commit message: " We got connection failures because of outdated channel information. That's because the NL80211_CMD_REG_CHANGE event is important for all interfaces. Commit f13683720239 ("nl80211: Pass wiphy events to all affected interfaces") skips the early termination for events directed to a wiphy, but that doesn't cover the regulatory change event because it doesn't have a wiphy set either. Therefore the early termination still kicks in and from three interfaces, only one got the updated channel list. Fix this by changing the early termination logic to only apply to events directed either to a specific interface index for wdev. " What do you think? Does it cover your concerns about context? Best regards Dominik Cermak | Software Engineer
On Thu, Aug 29, 2024 at 08:01:21AM +0000, Cermak Dominik wrote: > Extending commit f136837202393a7e1f3182e9efdbf1aaa0c1a5c2. > > We got connection failures because of outdated channel information. > That's because the NL80211_CMD_REG_CHANGE event is important for all > interfaces, but the early termination still kicks in because it is not > directed to a specific wiphy. > Therefore from three interfaces, only one got the updated channel list. > > Fix this by changing the early termination logic to only apply to events > directed either to a specific interface index for wdev. Thanks, applied.
diff --git a/src/drivers/driver_nl80211_event.c b/src/drivers/driver_nl80211_event.c index 903207067..8e79b5848 100644 --- a/src/drivers/driver_nl80211_event.c +++ b/src/drivers/driver_nl80211_event.c @@ -4257,7 +4257,13 @@ int process_global_event(struct nl_msg *msg, void *arg) wdev_id == bss->wdev_id)) { processed = true; do_process_drv_event(bss, gnlh->cmd, tb); - if (!wiphy_idx_set) + /* There are two types of events that may need to be + * delivered to multiple interfaces: + * 1. Events for a wiphy, as it can have multiple interfaces. + * 2. "Global" events, like NL80211_CMD_REG_CHANGE. + * Or in other words, terminate early only if it is + * directed to a specific interface or wdev. */ + if (ifidx != -1 || wdev_id_set) return NL_SKIP; /* The driver instance could have been removed, * e.g., due to NL80211_CMD_RADAR_DETECT event,
Extending commit f136837202393a7e1f3182e9efdbf1aaa0c1a5c2. We got connection failures because of outdated channel information. That's because the NL80211_CMD_REG_CHANGE event is important for all interfaces, but the early termination still kicks in because it is not directed to a specific wiphy. Therefore from three interfaces, only one got the updated channel list. Fix this by changing the early termination logic to only apply to events directed either to a specific interface index for wdev. Signed-off-by: Dominik Cermak <dominik.cermak@joynext.com> --- src/drivers/driver_nl80211_event.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)