2016-06-10 19:47:50

by Ben Greear

[permalink] [raw]
Subject: NL80211_ATTR_PAD question

I see this was added sometime recently: NL80211_ATTR_PAD

If another enum member is added, should it replace the PAD enum?

At the least, I think we need some comments about how this is to be dealt
with.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com



2016-06-10 22:40:07

by Ben Greear

[permalink] [raw]
Subject: Re: NL80211_ATTR_PAD question



On 06/10/2016 02:08 PM, Johannes Berg wrote:
> On Fri, 2016-06-10 at 12:47 -0700, Ben Greear wrote:
>> I see this was added sometime recently: NL80211_ATTR_PAD
>>
>> If another enum member is added, should it replace the PAD enum?
>
> No.
>
>> At the least, I think we need some comments about how this is to be
>> dealt with.
>>
>
> You simply ignore it :)

Please add a comment...it is normal behaviour to fill in 'pads' when
possible, and even when I looked up the commit message it wasn't exactly
clear what this member was for.

Even reading nla_put_u64_64bit offers no useful clues...I didn't follow
the call chain longer, probably somewhere it would start making sense,
but then again, this is netlink code, so who knows!

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2016-06-21 11:13:05

by Johannes Berg

[permalink] [raw]
Subject: Re: NL80211_ATTR_PAD question

On Mon, 2016-06-13 at 21:25 +0200, Arend van Spriel wrote:
>
> On 10-06-16 23:08, Johannes Berg wrote:
> > On Fri, 2016-06-10 at 12:47 -0700, Ben Greear wrote:
> > > I see this was added sometime recently:  NL80211_ATTR_PAD
> > >
> > > If another enum member is added, should it replace the PAD enum?
> >
> > No.
> >
> > > At the least, I think we need some comments about how this is to
> > > be
> > > dealt with.
> > >
> >
> > You simply ignore it :)
>
> So do you know why the caller of nla_put_u64_64bit() would need to
> specify the padattr. I mean, why is this not an generic attribute
> that
> netlink deals with internally. It has probably been discussed on the
> netdev list but I could not find the right thread.
>

Netlink attribute numbers are always specific to the (generic) netlink
family used. There was a discussion about using the attribute number 0
since that's reserved, but unfortunately it's only reserved in *most*
families, some actually use it (despite that being broken with libnl.)

johannes

2016-06-10 20:29:01

by Arend Van Spriel

[permalink] [raw]
Subject: Re: NL80211_ATTR_PAD question



On 10-06-16 21:47, Ben Greear wrote:
> I see this was added sometime recently: NL80211_ATTR_PAD
>
> If another enum member is added, should it replace the PAD enum?

Nope. This attribute is used as padattr by 64-bit aligned netlink
functions, ie. nla_put_u64_64bit().

> At the least, I think we need some comments about how this is to be dealt
> with.

> * @NL80211_ATTR_PAD: attribute used for padding for 64-bit alignment

This could indeed use a bit more explanation.

Regards,
Arend

2016-06-10 21:08:55

by Johannes Berg

[permalink] [raw]
Subject: Re: NL80211_ATTR_PAD question

On Fri, 2016-06-10 at 12:47 -0700, Ben Greear wrote:
> I see this was added sometime recently:  NL80211_ATTR_PAD
>
> If another enum member is added, should it replace the PAD enum?

No.

> At the least, I think we need some comments about how this is to be
> dealt with.
>

You simply ignore it :)

johannes

2016-06-13 19:25:18

by Arend Van Spriel

[permalink] [raw]
Subject: Re: NL80211_ATTR_PAD question



On 10-06-16 23:08, Johannes Berg wrote:
> On Fri, 2016-06-10 at 12:47 -0700, Ben Greear wrote:
>> I see this was added sometime recently: NL80211_ATTR_PAD
>>
>> If another enum member is added, should it replace the PAD enum?
>
> No.
>
>> At the least, I think we need some comments about how this is to be
>> dealt with.
>>
>
> You simply ignore it :)

So do you know why the caller of nla_put_u64_64bit() would need to
specify the padattr. I mean, why is this not an generic attribute that
netlink deals with internally. It has probably been discussed on the
netdev list but I could not find the right thread.

Regards,
Arend