Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Received-SPF: Pass (esa2.microchip.iphmx.com: domain of
  Horatiu.Vultur@microchip.com designates 198.175.253.82 as
  permitted sender) identity=mailfrom;
  client-ip=198.175.253.82; receiver=esa2.microchip.iphmx.com;
  envelope-from="Horatiu.Vultur@microchip.com";
  x-sender="Horatiu.Vultur@microchip.com";
  x-conformance=spf_only; x-record-type="v=spf1";
  x-record-text="v=spf1 mx a:ushub1.microchip.com
  a:smtpout.microchip.com a:mx1.microchip.iphmx.com
  a:mx2.microchip.iphmx.com include:servers.mcsv.net
  include:mktomail.com include:spf.protection.outlook.com ~all"
Received-SPF: None (esa2.microchip.iphmx.com: no sender
  authenticity information available from domain of
  postmaster@email.microchip.com) identity=helo;
  client-ip=198.175.253.82; receiver=esa2.microchip.iphmx.com;
  envelope-from="Horatiu.Vultur@microchip.com";
  x-sender="postmaster@email.microchip.com";
  x-conformance=spf_only
IronPort-SDR: kF1RWub9CXK6jgsNDuMyJeTx+jQyYj0BBM+qp3iaZc6Z3VJvJA8g6AyYGnKLzKWwj8fxBoby7V
 l/x4S3KXk4xCn5JTHE6gStzK/nhksXncpn7mbQYJwfWgoqXREWpfi7YdWvTPVsuGARz0tCw9/z
 UIOqbcrb0Ew9fZL5Uy3H32/cdv7aaWE4ItiWEmQoWYwOLcrOD4xc5dYnNnjeXC0gD44Cc3Xpbt
 yOmbLiG3ZUlxgTmrN7I22ayxTRpcATfE1YL0duemHcwbB8DZUH7tHCaH8MRQcRijcYLOfvZEQZ
 md0=
Date:   Thu, 29 Aug 2019 12:56:54 +0200
From:   Horatiu Vultur <horatiu.vultur@microchip.com>
To:     Jiri Pirko <jiri@resnulli.us>
CC:     <alexandre.belloni@bootlin.com>, <UNGLinuxDriver@microchip.com>,
        <davem@davemloft.net>, <andrew@lunn.ch>,
        <allan.nielsen@microchip.com>, <ivecera@redhat.com>,
        <f.fainelli@gmail.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
Message-ID: <20190829105650.btgvytgja63sx6wx@soft-dev3.microsemi.net>
References: <1567070549-29255-1-git-send-email-horatiu.vultur@microchip.com>
 <1567070549-29255-2-git-send-email-horatiu.vultur@microchip.com>
 <20190829095100.GH2312@nanopsycho>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <20190829095100.GH2312@nanopsycho>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

The 08/29/2019 11:51, Jiri Pirko wrote:
> External E-Mail
> 
> 
> Thu, Aug 29, 2019 at 11:22:28AM CEST, horatiu.vultur@microchip.com wrote:
> >Add the SWITCHDEV_ATTR_ID_PORT_PROMISCUITY switchdev notification type,
> >used to indicate whenever the dev promiscuity counter is changed.
> >
> >The notification doesn't use any switchdev_attr attribute because in the
> >notifier callbacks is it possible to get the dev and read directly
> >the promiscuity value.
> >
> >Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> >---
> > include/net/switchdev.h | 1 +
> > net/core/dev.c          | 9 +++++++++
> > 2 files changed, 10 insertions(+)
> >
> >diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> >index aee86a1..14b1617 100644
> >--- a/include/net/switchdev.h
> >+++ b/include/net/switchdev.h
> >@@ -40,6 +40,7 @@ enum switchdev_attr_id {
> > 	SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING,
> > 	SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED,
> > 	SWITCHDEV_ATTR_ID_BRIDGE_MROUTER,
> >+	SWITCHDEV_ATTR_ID_PORT_PROMISCUITY,
> > };
> > 
> > struct switchdev_attr {
> >diff --git a/net/core/dev.c b/net/core/dev.c
> >index 49589ed..40c74f2 100644
> >--- a/net/core/dev.c
> >+++ b/net/core/dev.c
> >@@ -142,6 +142,7 @@
> > #include <linux/net_namespace.h>
> > #include <linux/indirect_call_wrapper.h>
> > #include <net/devlink.h>
> >+#include <net/switchdev.h>
> > 
> > #include "net-sysfs.h"
> > 
> >@@ -7377,6 +7378,11 @@ static void dev_change_rx_flags(struct net_device *dev, int flags)
> > static int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
> > {
> > 	unsigned int old_flags = dev->flags;
> >+	struct switchdev_attr attr = {
> >+		.orig_dev = dev,
> >+		.id = SWITCHDEV_ATTR_ID_PORT_PROMISCUITY,
> >+		.flags = SWITCHDEV_F_DEFER,
> 

Hi Jiri,

> NACK
> 
> This is invalid usecase for switchdev infra. Switchdev is there for
> bridge offload purposes only.
> 
> For promiscuity changes, the infrastructure is already present in the
> code. See __dev_notify_flags(). it calls:
> call_netdevice_notifiers_info(NETDEV_CHANGE, &change_info.info)
> and you can actually see the changed flag in ".flags_changed".
Yes, you are right. But in case the port is part of a bridge and then
enable promisc mode by a user space application(tpcdump) then the drivers
will not be notified. The reason is that the dev->flags will still be the
same(because IFF_PROMISC was already set) and only promiscuity will be
changed.

One fix could be to call __dev_notify_flags() no matter when the
promisc is enable or disabled.

> 
> You just have to register netdev notifier block in your driver. Grep
> for: register_netdevice_notifier
> 
> 
> >+	};
> > 	kuid_t uid;
> > 	kgid_t gid;
> > 
> >@@ -7419,6 +7425,9 @@ static int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
> > 	}
> > 	if (notify)
> > 		__dev_notify_flags(dev, old_flags, IFF_PROMISC);
> >+
> >+	switchdev_port_attr_set(dev, &attr);
> >+
> > 	return 0;
> > }
> > 
> >-- 
> >2.7.4
> >
> 

-- 
/Horatiu