| Message ID | 20220920100518.19705-3-quic_vthiagar@quicinc.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: cfg80211/mac80211: capability advertisement infra for multi-hw abstraction under one wiphy | expand |
On Tue, 2022-09-20 at 15:35 +0530, Vasanthakumar Thiagarajan wrote: > > +++ b/include/uapi/linux/nl80211.h > @@ -2749,6 +2749,12 @@ enum nl80211_commands { > * When used with %NL80211_CMD_FRAME_TX_STATUS, indicates the ack RX > * timestamp. When used with %NL80211_CMD_FRAME RX notification, indicates > * the incoming frame RX timestamp. > + * > + * @NL80211_ATTR_MULTI_HW_MACS: nested attribute to send the hardware mac Not sure I'd call this multiple MACs? It's multiple devices in some sense, but from a spec POV at least, I'd think our NIC also has multiple MACs when it doesn't use this infrastructure. Might get a bit confusing? Maybe just stick to "multi_hw" or so? > +/** > + * nl80211_multi_hw_mac_attrs - multi-hw mac attributes > + * > + * @NL80211_MULTI_HW_MAC_ATTR_INVALID: invalid > + * @NL80211_MULTI_HW_MAC_ATTR_IDX: (u8) array index in wiphy @hw_chans to refer an > + * underlying hw mac for which the supported channel list is advertised. I'd prefer this to be primarily written from a userspace POV, so the whole "@hw_chans" etc isn't really right. Maybe say something like "(u8) multi-HW index used to refer to an underlying HW ...; internally the index of the wiphy's @hw_chans array." or so? > + * @NL80211_MULTI_HW_MAC_ATTR_FREQS: array of supported center frequencies FWIW, Jakub has started advertising for using the same attribute multiple times to have arrays, so you'd just have {NL80211_MULTI_HW_ATTR_FREQ: 2412}, {NL80211_MULTI_HW_ATTR_FREQ: 2417}, {NL80211_MULTI_HW_ATTR_FREQ: 2422}, etc. in the message. Not sure we want to try that here, but it'd also simplify splitting messages for dumps. > +static int nl80211_put_multi_hw_support(struct wiphy *wiphy, > + struct sk_buff *msg) > +{ > + struct nlattr *hw_macs, *hw_mac; > + struct nlattr *freqs; > + int i, c; > + > + if (!wiphy->num_hw) > + return 0; > + > + hw_macs = nla_nest_start(msg, NL80211_ATTR_MULTI_HW_MACS); > + if (!hw_macs) > + return -ENOBUFS; > + > + for (i = 0; i < wiphy->num_hw; i++) { > + hw_mac = nla_nest_start(msg, i + 1); > + if (!hw_mac) > + return -ENOBUFS; > + > + if (nla_put_u8(msg, NL80211_MULTI_HW_MAC_ATTR_IDX, i)) > + return -ENOBUFS; > + > + freqs = nla_nest_start(msg, > + NL80211_MULTI_HW_MAC_ATTR_FREQS); > + if (!freqs) > + return -ENOBUFS; > + > + for (c = 0; c < wiphy->hw_chans[i]->n_chans; c++) > + if (nla_put_u32(msg, c + 1, > + wiphy->hw_chans[i]->chans[c].center_freq)) > + return -ENOBUFS; Ah you used a nested array even. So the argument for using a real array would've been that it's smaller, but I guess with nested that argument goes way. Would you mind trying Jakub's preferred approach here and see how that works out? For the generator basically you'd just have hw_mac = nla_nest_start(); nla_put_u8(IDX, i) for (c = 0; c < ...; c++) nla_put_u32(MULTI_HW_ATTR_FREQ, ...->chans[c].center_freq); johannes
On 10/21/2022 5:43 PM, Johannes Berg wrote: > On Tue, 2022-09-20 at 15:35 +0530, Vasanthakumar Thiagarajan wrote: >> >> +++ b/include/uapi/linux/nl80211.h >> @@ -2749,6 +2749,12 @@ enum nl80211_commands { >> * When used with %NL80211_CMD_FRAME_TX_STATUS, indicates the ack RX >> * timestamp. When used with %NL80211_CMD_FRAME RX notification, indicates >> * the incoming frame RX timestamp. >> + * >> + * @NL80211_ATTR_MULTI_HW_MACS: nested attribute to send the hardware mac > > Not sure I'd call this multiple MACs? It's multiple devices in some > sense, but from a spec POV at least, I'd think our NIC also has multiple > MACs when it doesn't use this infrastructure. Might get a bit confusing? > > Maybe just stick to "multi_hw" or so? Yeah, I was not very comfortable calling it multiple MACs either. Sure, let me just stick to multi_hw. > >> +/** >> + * nl80211_multi_hw_mac_attrs - multi-hw mac attributes >> + * >> + * @NL80211_MULTI_HW_MAC_ATTR_INVALID: invalid >> + * @NL80211_MULTI_HW_MAC_ATTR_IDX: (u8) array index in wiphy @hw_chans to refer an >> + * underlying hw mac for which the supported channel list is advertised. > > I'd prefer this to be primarily written from a userspace POV, so the > whole "@hw_chans" etc isn't really right. Maybe say something like > > "(u8) multi-HW index used to refer to an underlying HW ...; internally > the index of the wiphy's @hw_chans array." > > or so? Sure, thanks. > >> + * @NL80211_MULTI_HW_MAC_ATTR_FREQS: array of supported center frequencies > > FWIW, Jakub has started advertising for using the same attribute > multiple times to have arrays, so you'd just have > > {NL80211_MULTI_HW_ATTR_FREQ: 2412}, > {NL80211_MULTI_HW_ATTR_FREQ: 2417}, > {NL80211_MULTI_HW_ATTR_FREQ: 2422}, > > etc. in the message. Not sure we want to try that here, but it'd also > simplify splitting messages for dumps. > we have to do that for every channel? let me check. > >> +static int nl80211_put_multi_hw_support(struct wiphy *wiphy, >> + struct sk_buff *msg) >> +{ >> + struct nlattr *hw_macs, *hw_mac; >> + struct nlattr *freqs; >> + int i, c; >> + >> + if (!wiphy->num_hw) >> + return 0; >> + >> + hw_macs = nla_nest_start(msg, NL80211_ATTR_MULTI_HW_MACS); >> + if (!hw_macs) >> + return -ENOBUFS; >> + >> + for (i = 0; i < wiphy->num_hw; i++) { >> + hw_mac = nla_nest_start(msg, i + 1); >> + if (!hw_mac) >> + return -ENOBUFS; >> + >> + if (nla_put_u8(msg, NL80211_MULTI_HW_MAC_ATTR_IDX, i)) >> + return -ENOBUFS; >> + >> + freqs = nla_nest_start(msg, >> + NL80211_MULTI_HW_MAC_ATTR_FREQS); >> + if (!freqs) >> + return -ENOBUFS; >> + >> + for (c = 0; c < wiphy->hw_chans[i]->n_chans; c++) >> + if (nla_put_u32(msg, c + 1, >> + wiphy->hw_chans[i]->chans[c].center_freq)) >> + return -ENOBUFS; > > Ah you used a nested array even. > > So the argument for using a real array would've been that it's smaller, > but I guess with nested that argument goes way. > > Would you mind trying Jakub's preferred approach here and see how that > works out? > > For the generator basically you'd just have > > hw_mac = nla_nest_start(); > nla_put_u8(IDX, i) > for (c = 0; c < ...; c++) > nla_put_u32(MULTI_HW_ATTR_FREQ, ...->chans[c].center_freq); > I'll try this, thanks! Vasanth
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index c32e7616a366..070b31277402 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -2749,6 +2749,12 @@ enum nl80211_commands { * When used with %NL80211_CMD_FRAME_TX_STATUS, indicates the ack RX * timestamp. When used with %NL80211_CMD_FRAME RX notification, indicates * the incoming frame RX timestamp. + * + * @NL80211_ATTR_MULTI_HW_MACS: nested attribute to send the hardware mac + * specific channel capabilities to user space. Drivers registering + * multiple physical hardware under a wiphy can use this attribute, + * see &enum nl80211_multi_hw_mac_attrs. + * * @NUM_NL80211_ATTR: total number of nl80211_attrs available * @NL80211_ATTR_MAX: highest attribute number currently defined * @__NL80211_ATTR_AFTER_LAST: internal use @@ -3277,6 +3283,8 @@ enum nl80211_attrs { NL80211_ATTR_TX_HW_TIMESTAMP, NL80211_ATTR_RX_HW_TIMESTAMP, + NL80211_ATTR_MULTI_HW_MACS, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, @@ -7720,4 +7728,24 @@ enum nl80211_ap_settings_flags { NL80211_AP_SETTINGS_SA_QUERY_OFFLOAD_SUPPORT = 1 << 1, }; +/** + * nl80211_multi_hw_mac_attrs - multi-hw mac attributes + * + * @NL80211_MULTI_HW_MAC_ATTR_INVALID: invalid + * @NL80211_MULTI_HW_MAC_ATTR_IDX: (u8) array index in wiphy @hw_chans to refer an + * underlying hw mac for which the supported channel list is advertised. + * @NL80211_MULTI_HW_MAC_ATTR_FREQS: array of supported center frequencies + * @__NL80211_MULTI_HW_MAC_ATTR_LAST: internal use + * @NL80211_MULTI_HW_MAC_ATTR_MAX: maximum multi-hw mac attribute + */ +enum nl80211_multi_hw_mac_attrs { + __NL80211_MULTI_HW_MAC_ATTR_INVALID, + + NL80211_MULTI_HW_MAC_ATTR_IDX, + NL80211_MULTI_HW_MAC_ATTR_FREQS, + + /* keep last */ + __NL80211_MULTI_HW_MAC_ATTR_LAST, + NL80211_MULTI_HW_MAC_ATTR_MAX = __NL80211_MULTI_HW_MAC_ATTR_LAST - 1 +}; #endif /* __LINUX_NL80211_H */ diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 8ff8b1c040f0..b7d466010e81 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -2355,6 +2355,47 @@ static int nl80211_put_mbssid_support(struct wiphy *wiphy, struct sk_buff *msg) return -ENOBUFS; } +static int nl80211_put_multi_hw_support(struct wiphy *wiphy, + struct sk_buff *msg) +{ + struct nlattr *hw_macs, *hw_mac; + struct nlattr *freqs; + int i, c; + + if (!wiphy->num_hw) + return 0; + + hw_macs = nla_nest_start(msg, NL80211_ATTR_MULTI_HW_MACS); + if (!hw_macs) + return -ENOBUFS; + + for (i = 0; i < wiphy->num_hw; i++) { + hw_mac = nla_nest_start(msg, i + 1); + if (!hw_mac) + return -ENOBUFS; + + if (nla_put_u8(msg, NL80211_MULTI_HW_MAC_ATTR_IDX, i)) + return -ENOBUFS; + + freqs = nla_nest_start(msg, + NL80211_MULTI_HW_MAC_ATTR_FREQS); + if (!freqs) + return -ENOBUFS; + + for (c = 0; c < wiphy->hw_chans[i]->n_chans; c++) + if (nla_put_u32(msg, c + 1, + wiphy->hw_chans[i]->chans[c].center_freq)) + return -ENOBUFS; + + nla_nest_end(msg, freqs); + + nla_nest_end(msg, hw_mac); + } + + nla_nest_end(msg, hw_macs); + return 0; +} + struct nl80211_dump_wiphy_state { s64 filter_wiphy; long start; @@ -2959,6 +3000,12 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_MLO) nla_put_flag(msg, NL80211_ATTR_MLO_SUPPORT); + state->split_start++; + break; + case 17: + if (nl80211_put_multi_hw_support(&rdev->wiphy, msg)) + goto nla_put_failure; + /* done */ state->split_start = 0; break;
When driver supports multiple physical hardware under one wiphy, wiphy->num_hw != 0, send per-hardware supported frequency list to user space. List of frequency are reported inside an index which identifies the hardware as in wiphy->hw_chans[]. This hardware index will be used in follow up patches to identify the interface combination capability for each of the underlying physical hardware abstracted under one wiphy. Signed-off-by: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com> --- include/uapi/linux/nl80211.h | 28 +++++++++++++++++++++ net/wireless/nl80211.c | 47 ++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+)