netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@idosch.org>
To: netdev@kapio-technology.com
Cc: davem@davemloft.net, kuba@kernel.org, netdev@vger.kernel.org,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Vladimir Oltean" <olteanv@gmail.com>,
	"Eric Dumazet" <edumazet@google.com>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Kurt Kanzenbach" <kurt@linutronix.de>,
	"Hauke Mehrtens" <hauke@hauke-m.de>,
	"Woojung Huh" <woojung.huh@microchip.com>,
	"maintainer:MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER"
	<UNGLinuxDriver@microchip.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Landen Chao" <Landen.Chao@mediatek.com>,
	"DENG Qingfang" <dqfext@gmail.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Claudiu Manoil" <claudiu.manoil@nxp.com>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Clément Léger" <clement.leger@bootlin.com>,
	"Jiri Pirko" <jiri@resnulli.us>,
	"Ivan Vecera" <ivecera@redhat.com>,
	"Roopa Prabhu" <roopa@nvidia.com>,
	"Nikolay Aleksandrov" <razor@blackwall.org>,
	"Russell King" <linux@armlinux.org.uk>,
	"Christian Marangi" <ansuelsmth@gmail.com>,
	"open list" <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-arm-kernel@lists.infradead.org>,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"open list:RENESAS RZ/N1 A5PSW SWITCH DRIVER"
	<linux-renesas-soc@vger.kernel.org>,
	"moderated list:ETHERNET BRIDGE"
	<bridge@lists.linux-foundation.org>
Subject: Re: [PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier
Date: Thu, 2 Feb 2023 18:11:59 +0200	[thread overview]
Message-ID: <Y9vgz4x/O+dIp+0/@shredder> (raw)
In-Reply-To: <f27dd18d9d0b7ff8b693af8a58ea8616@kapio-technology.com>

On Thu, Feb 02, 2023 at 08:28:36AM +0100, netdev@kapio-technology.com wrote:
> On 2023-02-01 19:10, Ido Schimmel wrote:
> > On Mon, Jan 30, 2023 at 06:34:25PM +0100, Hans J. Schultz wrote:
> > > To be able to add dynamic FDB entries to drivers from userspace, the
> > > dynamic flag must be added when sending RTM_NEWNEIGH events down.
> > > 
> > > Signed-off-by: Hans J. Schultz <netdev@kapio-technology.com>
> > > ---
> > >  include/net/switchdev.h   | 1 +
> > >  net/bridge/br_switchdev.c | 2 ++
> > >  2 files changed, 3 insertions(+)
> > > 
> > > diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> > > index ca0312b78294..aaf918d4ba67 100644
> > > --- a/include/net/switchdev.h
> > > +++ b/include/net/switchdev.h
> > > @@ -249,6 +249,7 @@ struct switchdev_notifier_fdb_info {
> > >  	u8 added_by_user:1,
> > >  	   is_local:1,
> > >  	   locked:1,
> > > +	   is_dyn:1,
> > >  	   offloaded:1;
> > >  };
> > > 
> > > diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> > > index 7eb6fd5bb917..4420fcbbfdb2 100644
> > > --- a/net/bridge/br_switchdev.c
> > > +++ b/net/bridge/br_switchdev.c
> > > @@ -136,6 +136,8 @@ static void br_switchdev_fdb_populate(struct
> > > net_bridge *br,
> > >  	item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
> > >  	item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
> > >  	item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
> > > +	item->is_dyn = !test_bit(BR_FDB_STATIC, &fdb->flags) &&
> > 
> > Why not 'is_static' and be consistent with the bridge flag like all the
> > other fields?
> > 
> > Regardless of how you name this field, it is irrelevant for
> > 'SWITCHDEV_FDB_ADD_TO_BRIDGE' notifications that all add FDB entries
> > with the 'BR_FDB_ADDED_BY_EXT_LEARN' flag set, which makes
> > 'BR_FDB_STATIC' irrelevant.
> > 
> > > +		item->added_by_user;
> > 
> > Unclear why this is needed...
> > 
> 
> The answer to those two questions lies in my earlier correspondences (with
> Oltean) on the RFC version.

It is not up to me as a reviewer to dig up old versions of the patch and
find out what was changed and why. It is up to you as the submitter of
the patch to provide all this information in the patch posting. Please
read:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

Specifically:

"Review comments or questions that do not lead to a code change should
almost certainly bring about a comment or changelog entry so that the
next reviewer better understands what is going on."

And:

"Other comments relevant only to the moment or the maintainer, not
suitable for the permanent changelog, should also go here. A good
example of such comments might be patch changelogs which describe what
has changed between the v1 and v2 version of the patch.

Please put this information after the --- line which separates the
changelog from the rest of the patch. The version information is not
part of the changelog which gets committed to the git tree. It is
additional information for the reviewers."

Thanks

  reply	other threads:[~2023-02-02 16:12 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-30 17:34 [PATCH net-next 0/5] ATU and FDB synchronization on locked ports Hans J. Schultz
2023-01-30 17:34 ` [PATCH net-next 1/5] net: bridge: add dynamic flag to switchdev notifier Hans J. Schultz
2023-02-01 18:10   ` Ido Schimmel
2023-02-02  7:28     ` netdev
2023-02-02 16:11       ` Ido Schimmel [this message]
2023-02-02 16:38         ` netdev
2023-02-03 16:14           ` Ido Schimmel
2023-02-03 16:26             ` Vladimir Oltean
2023-02-03 16:27             ` netdev
2023-02-03 17:06               ` Ido Schimmel
2023-01-30 17:34 ` [PATCH net-next 2/5] net: dsa: propagate flags down towards drivers Hans J. Schultz
2023-01-30 17:34 ` [PATCH net-next 3/5] drivers: net: dsa: add fdb entry flags incoming to switchcore drivers Hans J. Schultz
2023-01-31 18:54   ` Simon Horman
2023-02-02 16:45     ` netdev
2023-02-03  8:17       ` Simon Horman
2023-02-03 18:41         ` netdev
2023-01-30 17:34 ` [PATCH net-next 4/5] net: bridge: ensure FDB offloaded flag is handled as needed Hans J. Schultz
2023-02-01 18:24   ` Ido Schimmel
2023-02-02  7:32     ` netdev
2023-01-30 17:34 ` [PATCH net-next 5/5] net: dsa: mv88e6xxx: implementation of dynamic ATU entries Hans J. Schultz
2023-01-31 18:56   ` Simon Horman
2023-02-02 17:00     ` netdev
2023-02-03  8:20       ` Simon Horman
2023-02-03 20:44         ` Vladimir Oltean
2023-02-04  8:12           ` Simon Horman
2023-02-04  8:48             ` netdev
2023-02-06 16:02               ` Simon Horman
2023-02-14 21:14                 ` Hans Schultz
2023-02-17 17:44                   ` Vladimir Oltean
2023-02-20 14:11                     ` Simon Horman
2023-01-31 19:25 ` [PATCH net-next 0/5] ATU and FDB synchronization on locked ports Ido Schimmel
2023-02-02  7:37   ` netdev
2023-02-02 15:43     ` Ido Schimmel
2023-02-02 16:19       ` netdev
2023-02-02 16:36         ` Ido Schimmel
2023-02-03 21:14           ` Vladimir Oltean
2023-02-02 17:18   ` netdev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y9vgz4x/O+dIp+0/@shredder \
    --to=idosch@idosch.org \
    --cc=Landen.Chao@mediatek.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=ansuelsmth@gmail.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=claudiu.manoil@nxp.com \
    --cc=clement.leger@bootlin.com \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=hauke@hauke-m.de \
    --cc=ivecera@redhat.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=kurt@linutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@kapio-technology.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=roopa@nvidia.com \
    --cc=sean.wang@mediatek.com \
    --cc=woojung.huh@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).