more nl80211/iw tool code comments

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1761 bytes --]

Hi,

Just briefly looked at your iw tool. Code looks really good, very
readable and understandable :)

A few thoughts/comments:

get_phymode should probably be more strict and not accept things like
"IEEEabg" as A mode. IMHO the only valid things should be
"IEEE<space>802.11<single char>"
"802.11<single char>"
"<single char>"


get_iftype should accept "ap-vlan" which str_iftype returns, although
it's not really useful for use by hand anyway, I think.


You have

iw phy set      [ phy ] DEVICE [ CHANSPEC ] [ name NEWNAME ]

and we discussed on IRC that it might make sense to have the same for
nl80211. On the surface, that makes sense, however, it does add a
complication in that we need to either specify that you cannot combine
some attributes (which doesn't really make sense), or we need to take
quite a bit of care with atomicity; setting the channel and changing the
phy name can both fail individually but having them in one netlink
message implies that it's one transaction. I'm not sure the somewhat
cleaner API and saving one command number is worth the additional
transactional safety we need to be careful with then.

So I'd like to reverse my previously stated opinion and say that I now
think that putting these orthogonal things into different commands would
be better so that we don't run into this transaction problem.

Of course, that doesn't counter the other thing, namely that commands
could (should?) be named NL80211_CMD_WIPHY_NAME_{SET,GET,NEW} (del?) and
occur in groups etc.


Another thing: Maybe it should be possible to say "phy# 1" in addition
to "phy phy1" so that it's easier to write scripts that don't care about
concurrent phy name changes? Just a thought.




johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]