Re: [patch net-next v4 14/21] bridge: add brport flags to dflt bridge_getlink

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

On 11/27/14 15:46, Scott Feldman wrote:
> On Thu, Nov 27, 2014 at 3:17 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:

>
> For RTM_GETLINK, rtnl_bridge_getlink() calls ndo_bridge_getlink twice
> for each dev, once on bridge and second time on dev.  Each call adds
> an RTM_NEWLINK to skb.  For the ndo_bridge_getlink() call to bridge,
> the MASTER port flags are filled in using br_port_fill_attr().  For
> the second ndo_bridge_getlink() call to dev, the port driver calls
> ndo_dflt_bridge_getlink() which fills in the SELF port flags.  Before
> this patch, ndo_dflt_bridge_getlink() was only filling in hwmode.
>
> Whew, in any case, I think you'll agree this code needs a refactoring
> down the road.  This change is just the bare minimum building on
> what's there to get SELF port flags up to user-space.  A refactoring
> effort should get the port drivers out of parsing/filling netlink msg
> and leave that to the core code in rtnetlink.c.  That way we can have
> one place for policy checks and one place for fill.  I think this
> refactoring effort should be left out in this patch series, otherwise
> this is going to drag on into the next year.
>

I am fine with that. At minimal  br_port_fill_attr() is reusable.

There's a lot of stuff i wish would be "fixed" - one is clearly
not abusing a u8 just to send one bit to the kernel. You just
added one more horn of that sort with the sync learning.
I wish i had time to clean it up. In any case:

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal