Re: [PATCH net-next 3/3] macsec: introduce IEEE 802.1AE driver

[Johannes Berg <johannes@sipsolutions.net>]

Hi,

Thanks for the comments.

> > > +struct gcm_iv {
> > > +	union {
> > > +		u8 secure_channel_id[8];
> > > +		sci_t sci;
> > > +	};
> > > +	__be32 pn;
> > > +};
> > 
> > Should this be __packed?
> 
> I think that's not necessary here.

Yeah, there's probably no way a compiler could ever do something with
it that's not the same as packed, but it seems to me that just out of
convention structs that have some wire-format meaning should usually be
__packed. But it's your call. I'm not even entirely sure it has a wire-
format or similar meaning, although the name indicates it gets used
into the encryption and must be exactly these 12 octets.

> > That way, you have the same policy for everything and also don't
> > have
> > to play tricks with the aliasing since the top-level attributes
> > actually exist now, coming from the same namespace & policy.
> 
> That's a good idea, I'll have a look today.
> I don't really like the aliasing games I have to play, but I'd like
> to
> keep the RXSC attributes separate from the SA attributes, I think it
> looks cleaner (the first RFC had everything in the same policy:
> http://www.spinics.net/lists/netdev/msg358152.html).
> 

Ah, I see. Keeping it separate makes sense, but you can still achieve
that like this:

msg = [ifindex -> 7,
       rxsc -> [ sci -> 2, ...
               ],
       ...
      ]

with nested data. I'd also do the same for the stats and a whole bunch
of others too, I guess. I tend to imagine it as a kind of "dict of
dicts" (the message with nested).

Thanks,
johannes