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
