Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755169AbbHFOTk (ORCPT ); Thu, 6 Aug 2015 10:19:40 -0400 Received: from mail.savoirfairelinux.com ([209.172.62.77]:64273 "EHLO mail.savoirfairelinux.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755074AbbHFOTh (ORCPT ); Thu, 6 Aug 2015 10:19:37 -0400 Date: Thu, 6 Aug 2015 10:19:34 -0400 From: Vivien Didelot To: Scott Feldman Cc: Netdev , "linux-kernel@vger.kernel.org" , kernel , "David S. Miller" , Guenter Roeck , Andrew Lunn , Florian Fainelli , Jiri Pirko Subject: Re: [PATCH net-next v2 2/7] net: switchdev: support static FDB addresses Message-ID: <20150806141934.GA20660@ketchup.touchtunes.com> References: <1438839848-505-1-git-send-email-vivien.didelot@savoirfairelinux.com> <1438839848-505-3-git-send-email-vivien.didelot@savoirfairelinux.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2354 Lines: 60 On 15-08-05 23:28:15, Scott Feldman wrote: > On Wed, Aug 5, 2015 at 10:44 PM, Vivien Didelot > wrote: > > This patch adds a is_static boolean to the switchdev_obj_fdb structure, > > in order to set the ndm_state to either NUD_NOARP or NUD_REACHABLE. > > > > Signed-off-by: Vivien Didelot > > --- > > include/net/switchdev.h | 1 + > > net/switchdev/switchdev.c | 2 +- > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/include/net/switchdev.h b/include/net/switchdev.h > > index e90e1a0..0e296b8 100644 > > --- a/include/net/switchdev.h > > +++ b/include/net/switchdev.h > > @@ -72,6 +72,7 @@ struct switchdev_obj { > > struct switchdev_obj_fdb { /* PORT_FDB */ > > u8 addr[ETH_ALEN]; > > u16 vid; > > + bool is_static; > > What do you think about changing this to u16 ndm_state? That way, it > can be used on input (fdb add) and output (fdb dump), and the driver > can privately track the state, kind of like how the bridge keeps > is_static, is_local, etc. I'm OK with the change. Should we consider NUD_NONE (0) a valid value? > > } fdb; > > } u; > > }; > > diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c > > index 9db87a3..e9d1cac 100644 > > --- a/net/switchdev/switchdev.c > > +++ b/net/switchdev/switchdev.c > > @@ -811,7 +811,7 @@ static int switchdev_port_fdb_dump_cb(struct net_device *dev, > > ndm->ndm_flags = NTF_SELF; > > ndm->ndm_type = 0; > > ndm->ndm_ifindex = dev->ifindex; > > - ndm->ndm_state = NUD_REACHABLE; > > + ndm->ndm_state = obj->u.fdb.is_static ? NUD_NOARP : NUD_REACHABLE; In other word, do we prefer this: ndm->ndm_state = obj->u.fdb.ndm_state == NUD_NONE ? NUD_REACHABLE : obj->u.fdb.ndm_state; Or this (meaning switchdev users cannot leave it blank and must at least set NUD_REACHABLE themselves): ndm->ndm_state = obj->u.fdb.ndm_state; Thanks, -v -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/