[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 12 Dec 2014 13:36:41 -0000
From: "Hubert Sokolowski" <h.sokolowski@....edu.pl>
To: "Jamal Hadi Salim" <jhs@...atatu.com>
Cc: "Hubert Sokolowski" <h.sokolowski@....edu.pl>,
"Roopa Prabhu" <roopa@...ulusnetworks.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"Vlad Yasevich" <vyasevic@...hat.com>
Subject: Re: [PATCH net-next RESEND] net: Do not call ndo_dflt_fdb_dump if
ndo_fdb_dump is defined.
> On 12/12/14 06:38, Hubert Sokolowski wrote:
>>> On 12/11/14, 9:06 AM, Hubert Sokolowski wrote:
>
>> Please see how the ndo_dflt_fdb_add and del are called:
>> if (dev->netdev_ops->ndo_fdb_add)
>> err = dev->netdev_ops->ndo_fdb_add(ndm, tb, dev, addr,
>> vid,
>> nlh->nlmsg_flags);
>> else
>> err = ndo_dflt_fdb_add(ndm, tb, dev, addr, vid,
>> nlh->nlmsg_flags);
>>
>
> Semantically add and dump are not the same.
> Add adds a specific entry.
> Dump not only dumps the fdb table but also the unicast and multicast
> addresses.
this is not true for 3.16 and before. Please see:
http://lxr.free-electrons.com/source/net/core/rtnetlink.c?v=3.16#L2545
It lets you fully manage the FDB table by overwriding fdb_add, del and dump
in the same way.
>
>
>> As it was suggested by Ronen I can modify the patch so the dflt callback
>> is always called for bridge devices if this is desirable. Either by calling
>> it when following expression is true:
>> (dev->priv_flags & IFF_BRIDGE_PORT)
>> or by modifying br_fdb_dump to call ndo_dflt_fdb_dump.
>>
>
> Are you saying the above is going to work? You need to TEST please.
yes, it works and it is not a rocket science :). we just need to agree
on the approach to use.
>
> It seems to me drivers which do this:
> ---
> .ndo_fdb_dump = my_fdb_dump,
>
> and then my_fdb_dump is:
> return ndo_dflt_fdb_dump(skb, ncb, netdev, filter_dev, idx);
>
> are broken because we always have to dump the uni/multicast
> addresses *unconditionally* and these drivers claim to be
> overriding the dumper but are in fact calling the default dumper.
> They are not filtering anything as you had stated.
> I wish Vlad (Cced) would show up and say something ;->
>
> IOW, fix all the broken driver to not do that.
These "broken" drivers were written before 3.17 when the unconditional call
to dflt_dump was introduced. Some drivers implement own dump and don't
call dflt_dump at all, some call dflt_dump depending on some condition.
You can also call dflt_dump inside, save the idx result and continue
with own dump resulting in both uc/mc and own entries returned to the user.
thanks,
Hubert
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists