Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755444AbbHFQhf (ORCPT ); Thu, 6 Aug 2015 12:37:35 -0400 Received: from mail-ob0-f172.google.com ([209.85.214.172]:36300 "EHLO mail-ob0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755220AbbHFQhd (ORCPT ); Thu, 6 Aug 2015 12:37:33 -0400 MIME-Version: 1.0 In-Reply-To: <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> <20150806141934.GA20660@ketchup.touchtunes.com> From: Scott Feldman Date: Thu, 6 Aug 2015 09:37:12 -0700 Message-ID: Subject: Re: [PATCH net-next v2 2/7] net: switchdev: support static FDB addresses To: Vivien Didelot Cc: Netdev , "linux-kernel@vger.kernel.org" , kernel , "David S. Miller" , Guenter Roeck , Andrew Lunn , Florian Fainelli , Jiri Pirko Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2530 Lines: 61 On Thu, Aug 6, 2015 at 7:19 AM, Vivien Didelot wrote: > 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; My preference would be this option. -- 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/