linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Horatiu Vultur <horatiu.vultur@microchip.com>
To: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Cc: <davem@davemloft.net>, <jiri@resnulli.us>, <ivecera@redhat.com>,
	<kuba@kernel.org>, <roopa@cumulusnetworks.com>,
	<olteanv@gmail.com>, <andrew@lunn.ch>,
	<UNGLinuxDriver@microchip.com>, <linux-kernel@vger.kernel.org>,
	<netdev@vger.kernel.org>, <bridge@lists.linux-foundation.org>
Subject: Re: [PATCH net-next 04/13] bridge: mrp: Expose function br_mrp_port_open
Date: Mon, 20 Apr 2020 21:26:54 +0200	[thread overview]
Message-ID: <20200420192654.ivar7apyv5xnzwg3@soft-dev3.microsemi.net> (raw)
In-Reply-To: <cc388b70-701c-4600-791b-8170683a7666@cumulusnetworks.com>

The 04/20/2020 20:30, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 20/04/2020 18:09, Horatiu Vultur wrote:
> > In case the HW is capable to detect when the MRP ring is open or closed. It is
> > expected that the network driver will notify the SW that the ring is open or
> > closed.
> >
> > The function br_mrp_port_open is used to notify the kernel that one of the ports
> > stopped receiving MRP_Test frames. The argument 'loc' has a value of '1' when
> > the port stopped receiving MRP_Test and '0' when it started to receive MRP_Test.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  include/linux/mrp_bridge.h | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >  create mode 100644 include/linux/mrp_bridge.h
> >
> > diff --git a/include/linux/mrp_bridge.h b/include/linux/mrp_bridge.h
> > new file mode 100644
> > index 000000000000..61c1f0d395c7
> > --- /dev/null
> > +++ b/include/linux/mrp_bridge.h
> > @@ -0,0 +1,27 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#ifndef _LINUX_MRP_BRIDGE_H
> > +#define _LINUX_MRO_BRIDGE_H
> > +
> > +#include <linux/netdevice.h>
> > +
> > +/* The drivers are responsible to call this function when it detects that the
> > + * MRP port stopped receiving MRP_Test frames or it started to receive MRP_Test.
> > + * The argument dev represents the port and loc(Lost of Continuity) has a value
> > + * of 1 when it stopped receiving MRP_Test frames and a value of 0 when it
> > + * started to receive frames.
> > + * Needs to be called with rcu_read_lock().
> > + *
> > + * This eventually notify the userspace which is required to react on these
> > + * changes.
> > + */
> > +
> > +#if IS_ENABLED(CONFIG_BRIDGE_MRP)
> > +int br_mrp_port_open(struct net_device *dev, u8 loc);
> > +#else
> > +static inline int br_mrp_port_open(struct net_device *dev, u8 loc)
> > +{
> > +}
> > +#endif
> > +
> > +#endif
> >

Hi Nik,
> 
> I see that in order to pass this "loc" to br_ifinfo_notify() you add a new port variable,
> but it's a boolean, so you can just use the port flags for it which will simplify things a lot
> and remove some ifdefs along the way. And please rename it to something longer (so it's easier
> to get what it does).

I will update to use the port flags.

> 
> One question - now that it's always sent to user-space as IFLA_BRPORT_MRP_RING_OPEN attribute
> wouldn't it be a problem, or is it read as a current state?
> I mean you'll receive that attribute about a port even if you get a completely unrelated to MRP
> notification (e.g. about a vlan change).

It should not be a problem. The default value is 0, which means that the
port has not lost the continuity.
So let's say that a port lost the continuity then the userspace will
receive a call where attribute IFLA_BRPORT_MRP_RING_OPEN will have the
value 1. This is as expected.
Then as you suggested, there is a vlan change, the userspace will
receive a notification but again the attribute IFLA_BRPORT_MRP_RING_OPEN
is 1, because the actual value is stored in the port variable, or port
flags in the new patch). But the userspace can detect this case and just
ignores it.
The same will apply also if the userspace will receive multiple
notification if the port didn't lose the continuity.

> 
> Thanks,
>  Nik

-- 
/Horatiu

  reply	other threads:[~2020-04-20 19:26 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-20 15:09 [PATCH net-next 00/13] net: bridge: mrp: Add support for Media Redundancy Protocol(MRP) Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 01/13] bridge: uapi: mrp: Add mrp attributes Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 02/13] net: bridge: Add port attribute IFLA_BRPORT_MRP_RING_OPEN Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 03/13] bridge: mrp: Update Kconfig Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 04/13] bridge: mrp: Expose function br_mrp_port_open Horatiu Vultur
2020-04-20 17:30   ` Nikolay Aleksandrov
2020-04-20 19:26     ` Horatiu Vultur [this message]
2020-04-20 15:09 ` [PATCH net-next 05/13] bridge: mrp: Add MRP interface Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 06/13] bridge: mrp: Extend bridge interface Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 07/13] switchdev: mrp: Extend switchdev API to offload MRP Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 08/13] bridge: switchdev: mrp: Implement MRP API for switchdev Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 09/13] bridge: mrp: Connect MRP API with the switchev API Horatiu Vultur
2020-04-20 17:17   ` Nikolay Aleksandrov
2020-04-20 19:13     ` Horatiu Vultur
2020-04-21  8:44   ` Nikolay Aleksandrov
2020-04-21  9:31     ` Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 10/13] bridge: mrp: Implement netlink interface to configure MRP Horatiu Vultur
2020-04-20 17:18   ` Nikolay Aleksandrov
2020-04-20 19:16     ` Horatiu Vultur
2020-04-21  8:47   ` Nikolay Aleksandrov
2020-04-21  9:17     ` Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 11/13] bridge: mrp: Update Makefile Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 12/13] bridge: mrp: Integrate MRP into the bridge Horatiu Vultur
2020-04-20 15:09 ` [PATCH net-next 13/13] net: bridge: Add checks for enabling the STP Horatiu Vultur

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=20200420192654.ivar7apyv5xnzwg3@soft-dev3.microsemi.net \
    --to=horatiu.vultur@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=ivecera@redhat.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=olteanv@gmail.com \
    --cc=roopa@cumulusnetworks.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).