Vivien Didelot <vivien.didelot@xxxxxxxxxxxxxxxxxxxx> writes:
> Hi Petr,
>
> Petr Machata <petrm@xxxxxxxxxxxx> writes:
>
>> Vivien Didelot <vivien.didelot@xxxxxxxxxxxxxxxxxxxx> writes:
>>
>>>> + } else {
>>>> + err = br_switchdev_port_obj_add(dev, v->vid, flags);
>>>> + if (err && err != -EOPNOTSUPP)
>>>> + goto out;
>>>> }
>>>
>>> Except that br_switchdev_port_obj_add taking vid and flags arguments
>>> seems confusing to me, the change looks good:
>>
>> I'm not sure what you're aiming at. Both VID and flags are sent with the
>> notification, so they need to be passed on to the function somehow. Do
>> you have a counterproposal for the API?
>
> I'm only questioning the code organization here, not the functional
> aspect which I do agree with. What I'm saying is that you name a new
> switchdev helper br_switchdev_port_OBJ_add, which takes VLAN arguments
> (vid and flags.) How would you call another eventual helper taking MDB
> arguments, br_switchdev_port_OBJ_add again? So something like
> br_switchdev_port_VLAN_add would be more intuitive.
>
> At the same time there's an effort to centralize all switchdev helpers
> of the bridge layer (i.e. the software -> hardware bridge calls) into
> net/bridge/br_switchdev.c, so that file would be more adequate.
>
> You may discard my comments but I think it'd be beneficial to us all to
> finally keep a bit of consistency in that bridge layer code.
Nope, those are reasonable points. I'll post a v2 along those lines.
Thanks,
Petr
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
