On Fri, 2016-04-15 at 16:57 +0530, Kanchanapally, Vidyullatha wrote: > > +struct wiphy_iftype_ext_capab { > + enum nl80211_iftype iftype; > + const u8 *ext_capab; > + const u8 *ext_capab_mask; > + u8 ext_capab_len;
I think you should reuse the struct member names that we used before - that will make grepping for them easier. > + struct wiphy_iftype_ext_capab *iftype_ext_capab; const > + * @NL80211_ATTR_IFTYPE_EXT_CAPA: Nested attribute of the following > attributes: > + * %NL80211_ATTR_IFTYPE, %NL80211_ATTR_EXT_CAPA, > + * %NL80211_ATTR_EXT_CAPA_MASK, to specify the extended > capabilities per > + * interface type. That's a bit awkward to parse, since you have to parse the big attribute list again, but I guess it still makes more sense than having entirely different attributes. > + state->split_start++; > + break; > + case 13: > + if (rdev->wiphy.num_iftype_ext_capab && > + rdev->wiphy.iftype_ext_capab) { > + struct nlattr *nested_ext_capab, *nested; > + > + nested = nla_nest_start(msg, > + NL80211_ATTR_IFTYPE_ > EXT_CAPA); > + if (!nested) > + goto nla_put_failure; > + > + for (i = 0; i < rdev- > >wiphy.num_iftype_ext_capab; i++) { > + struct wiphy_iftype_ext_capab > *capab; > + > + capab = &rdev- > >wiphy.iftype_ext_capab[i]; > + nested_ext_capab = > nla_nest_start(msg, i); > + if (!nested_ext_capab || > + nla_put_u32(msg, > NL80211_ATTR_IFTYPE, > + capab->iftype) || > + nla_put(msg, > NL80211_ATTR_EXT_CAPA, > + capab->ext_capab_len, > + capab->ext_capab) || > + nla_put(msg, > NL80211_ATTR_EXT_CAPA_MASK, > + capab->ext_capab_len, > + capab->ext_capab_mask)) > + goto nla_put_failure; > + > + nla_nest_end(msg, nested_ext_capab); > + } > + nla_nest_end(msg, nested); > + } I think we should consider allowing this to be split multiple messages (for different interface types), since this can potentially get rather long (interface types x 2 x capa len). OTOH, we'd have to get a LOT of interface types x capabilities to hit the limit of 4k, not sure. But it should just require a few lines of code to do this. Also, if it's possible, perhaps we should consider checking that nobody globally advertises any capabilities they don't advertise on all possible interface types? I'm assuming that not listing an interface type would mean that the global defaults apply, but old userspace will also not know about the per-interface, so you shouldn't list anything there. So I'm thinking something like supported_on_all = iftype_ext_capab[0] for i in 1..num_iftype_ext_capab-1: supported_on_all &= iftype_ext_capab[i] WARN_ON(wiphy->ext_capa_mask & ~supported_on_all) (which is obviously some kind of pseudo-code.) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html