Re: [net-next PATCH v2 1/5] introduce IFE action

[Jamal Hadi Salim <jhs@mojatatu.com>]

On 16-02-24 12:37 PM, Daniel Borkmann wrote:
> On 02/23/2016 01:49 PM, Jamal Hadi Salim wrote:
>> From: Jamal Hadi Salim <jhs@mojatatu.com>
> [...]
>> +static const struct nla_policy ife_policy[TCA_IFE_MAX + 1] = {
>> +    [TCA_IFE_PARMS] = {.len = sizeof(struct tc_ife)},
>> +    [TCA_IFE_DMAC] = {.type = NLA_BINARY,.len = ETH_ALEN},
>> +    [TCA_IFE_SMAC] = {.type = NLA_BINARY,.len = ETH_ALEN},
>
> This is buggy btw ...
>

I am sure i cutnpasted that from somewhere. Thanks for catching
it; I will remove NLA_BINARY ref.

>> +    [TCA_IFE_TYPE] = {.type = NLA_U16},
>> +};
>
> [...]
>
>> +    if (parm->flags & IFE_ENCODE) {
>> +        ife_type = *(u16 *) nla_data(tb[TCA_IFE_TYPE]);
>
> ( We have accessors for such things. Please also check coding style
>    and white space things in your series, there's couple of things all
>    over the place. )


Modern git tells you about white spaces - maybe i didnt stare long
enough ;-> I will use the accessor in next update.


> Maybe try to make this lockless in the fast path? Otherwise placing
> this on ingress / egress (f.e. clsact) doesn't really scale.
>


Let me think about it. Likely it will be subsequent patches - I just
want to get this set out first.

Thanks Daniel.

cheers,
jamal