From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikolay Aleksandrov Subject: Re: [PATCH net-next 2/2] net: bridge: Notify about !added_by_user FDB entries Date: Tue, 1 May 2018 20:49:15 +0300 Message-ID: References: <8934024270c83055e1e0e9468aafa3fe5e35e745.1525194039.git.petrm@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: ivecera@redhat.com, davem@davemloft.net, stephen@networkplumber.org, andrew@lunn.ch, vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com, jiri@resnulli.us, stephen@networkplumber.org, "bridge@lists.linux-foundation.org" To: Petr Machata , netdev@vger.kernel.org Return-path: Received: from mail-wm0-f67.google.com ([74.125.82.67]:36409 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932090AbeEARtS (ORCPT ); Tue, 1 May 2018 13:49:18 -0400 Received: by mail-wm0-f67.google.com with SMTP id n10so20080306wmc.1 for ; Tue, 01 May 2018 10:49:18 -0700 (PDT) In-Reply-To: <8934024270c83055e1e0e9468aafa3fe5e35e745.1525194039.git.petrm@mellanox.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 01/05/18 20:04, Petr Machata wrote: > Do not automatically bail out on sending notifications about activity on > non-user-added FDB entries. Instead, notify about this activity except > for cases where the activity itself originates in a notification, to > avoid sending duplicate notifications. > > Signed-off-by: Petr Machata > --- > net/bridge/br.c | 4 ++-- > net/bridge/br_fdb.c | 40 +++++++++++++++++++++++++++------------- > net/bridge/br_private.h | 4 ++-- > net/bridge/br_switchdev.c | 2 +- > 4 files changed, 32 insertions(+), 18 deletions(-) > Hi Petr, We already have 7 different fdb delete functions, I'm really not a fan of adding yet another one for such trivial change. Why don't you just add the new notify parameter to the already existing fdb_delete() ? (actually about the name see below) IMO it's confusing - if one wants a notification then use fdb_delete() or __fdb_delete(true) vs __fdb_delete(false) if a notification is not required. I think simply having the last parameter everywhere for fdb_delete() shows the intention clearer and avoids another fdb delete function. Another point, the notify parameter has a confusing name in this context because you're controlling the switchdev notifications not the rtnetlink ones. I'd suggest changing the name to something more descriptive like swdev_notify, otherwise you could get the funny end result of calling __fdb_notify() with notify == false which to me means don't notify. :-) Also please add the bridge maintainers to the CC list. Thanks, Nik