netdev - RE: [PATCH] bridge: Enable configuration of ageing interval for bridges and switch devices.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <91179B48C1F7014B86296413F181061E3D56B6E1@FMSMSX114.amr.corp.intel.com>
Date:	Wed, 19 Aug 2015 18:02:58 +0000
From:	"Wilson, Daniel G" <daniel.wilson@...el.com>
To:	Scott Feldman <sfeldma@...il.com>,
	Premkumar Jonnala <pjonnala@...adcom.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH] bridge: Enable configuration of ageing interval for
 bridges and switch devices.

> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
> On Behalf Of Scott Feldman
> Sent: Wednesday, August 19, 2015 12:54 PM
> To: Premkumar Jonnala
> Cc: netdev@...r.kernel.org
> Subject: Re: [PATCH] bridge: Enable configuration of ageing interval for bridges
> and switch devices.
> 
> On Wed, Aug 19, 2015 at 2:34 AM, Premkumar Jonnala
> <pjonnala@...adcom.com> wrote:
> > Hello Scott,
> >
> > Thank you for the diff and comments.   Please see my comments inline.
> >
> >> -----Original Message-----
> >> From: Scott Feldman [mailto:sfeldma@...il.com]
> >> Sent: Tuesday, August 18, 2015 12:48 PM
> >> To: Premkumar Jonnala
> >> Cc: netdev@...r.kernel.org
> >> Subject: Re: [PATCH] bridge: Enable configuration of ageing interval
> >> for bridges and switch devices.
> >>
> >>
> >>
> >> On Fri, 14 Aug 2015, Premkumar Jonnala wrote:
> >>
> >> > Bridge devices have ageing interval used to age out MAC addresses
> >> > from FDB.  This ageing interval was not configuratble.
> >> >
> >> > Enable netlink based configuration of ageing interval for bridges
> >> > and switch devices.  The ageing interval changes the timer used to
> >> > purge inactive FDB entries in bridges.  The ageing interval config
> >> > is propagated to switch devices, so that platform or hardware based
> >> > ageing works according to configuration.
> >> >
> >> > Signed-off-by: Premkumar Jonnala <pjonnala@...adcom.com>
> >>
> >> Hi Premkumar,
> >>
> >> I agree with Roopa that we should use existing IFLA_BR_AGEING_TIME.
> >
> > What is the motivation for using 'ip link' command to configure bridge
> > attributes?  IMHO, bridge command is better suited for that.
> 
> Can you extend bridge command to allow setting/getting these bridge attrs?
> Looks like you construct a RTM_NEWLINK IFLA_INFO_DATA msg.  No changes
> needed to the kernel.
> 
> bridge link set dev br0 ageing_time 1000
> 
>  --or--
> 
> ip link set dev br0 type bridge ageing_time 1000

Being able to set these attributes via both bridge and ip would be great.

> >> +int br_set_ageing_time(struct net_bridge *br, u32 ageing_time) {
> >> +     struct switchdev_attr attr = {
> >> +             .id = SWITCHDEV_ATTR_BRIDGE,
> >> +             .flags = SWITCHDEV_F_SKIP_EOPNOTSUPP,
> >> +             .u.bridge.attr = IFLA_BR_AGEING_TIME,
> >> +             .u.bridge.val = ageing_time,
> >> +     };
> >> +     int err;
> >> +
> >> +     err = switchdev_port_attr_set(br->dev, &attr);
> >> +     if (err)
> >> +             return err;
> >> +
> >> +     br->ageing_time = clock_t_to_jiffies(ageing_time);
> >
> > Should we restart the timer here the new time takes effect?
> 
> I don't know...I just copied what the original code did.  If it does need to be
> restarted, break that out as a separate patch.

In my opinion, yes, the timer should be restarted. If the timer had been set to 1 million seconds and is being changed to 1 minute, you wouldn't want to wait for the 1-million-second  timer to expire before resetting it to the newly-configured 1-minute timer value.

Dan.