Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net v2] net: phy: fix WoL handling when suspending the PHY
From: Heiner Kallweit @ 2018-09-23 13:39 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org
In-Reply-To: <f8737cef-88d2-0f60-bc96-88251e77c1bd@gmail.com>

Sorry, this patch was accidentally sent as reply.

On 23.09.2018 15:33, Heiner Kallweit wrote:
> Actually there's nothing wrong with the two changes marked as "Fixes",
> they just revealed a problem which has been existing before.
> After having switched r8169 to phylib it was reported that WoL from
> shutdown doesn't work any longer (WoL from suspend isn't affected).
> Reason is that during shutdown phy_disconnect()->phy_detach()->
> phy_suspend() is called.
> A similar issue occurs when the phylib state machine calls
> phy_suspend() when handling state PHY_HALTED.
> 
> Core of the problem is that phy_suspend() suspends the PHY when it
> should not due to WoL. phy_suspend() checks for WoL already, but this
> works only if the PHY driver handles WoL (what is rarely the case).
> Typically WoL is handled by the MAC driver.
> 
> phylib knows about this and handles it in mdio_bus_phy_may_suspend(),
> but that's used only when suspending the system, not in other cases
> like shutdown.
> 
> Therefore factor out the relevant check from
> mdio_bus_phy_may_suspend() to a new function phy_may_suspend() and
> use it in phy_suspend().
> 
> Last but not least change phy_detach() to call phy_suspend() before
> attached_dev is set to NULL. phy_suspend() accesses attached_dev
> when checking whether the MAC driver activated WoL.
> 
> Fixes: f1e911d5d0df ("r8169: add basic phylib support")
> Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - improved commit message
> - reduced scope of patch, don't touch functionality of
>   mdio_bus_phy_suspend and mdio_bus_phy_resume
> ---
>  drivers/net/phy/phy_device.c | 42 ++++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index af64a9320..4cab94bae 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -75,6 +75,26 @@ extern struct phy_driver genphy_10g_driver;
>  static LIST_HEAD(phy_fixup_list);
>  static DEFINE_MUTEX(phy_fixup_lock);
>  
> +static bool phy_may_suspend(struct phy_device *phydev)
> +{
> +	struct net_device *netdev = phydev->attached_dev;
> +
> +	if (!netdev)
> +		return true;
> +
> +	/* Don't suspend PHY if the attached netdev parent may wakeup.
> +	 * The parent may point to a PCI device, as in tg3 driver.
> +	 */
> +	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
> +		return false;
> +
> +	/* Also don't suspend PHY if the netdev itself may wakeup. This
> +	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
> +	 * e.g. SoC devices.
> +	 */
> +	return !device_may_wakeup(&netdev->dev);
> +}
> +
>  #ifdef CONFIG_PM
>  static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
>  {
> @@ -93,20 +113,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
>  	if (!netdev)
>  		return !phydev->suspended;
>  
> -	/* Don't suspend PHY if the attached netdev parent may wakeup.
> -	 * The parent may point to a PCI device, as in tg3 driver.
> -	 */
> -	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
> -		return false;
> -
> -	/* Also don't suspend PHY if the netdev itself may wakeup. This
> -	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
> -	 * e.g. SoC devices.
> -	 */
> -	if (device_may_wakeup(&netdev->dev))
> -		return false;
> -
> -	return true;
> +	return phy_may_suspend(phydev);
>  }
>  
>  static int mdio_bus_phy_suspend(struct device *dev)
> @@ -1132,9 +1139,9 @@ void phy_detach(struct phy_device *phydev)
>  		sysfs_remove_link(&dev->dev.kobj, "phydev");
>  		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
>  	}
> +	phy_suspend(phydev);
>  	phydev->attached_dev->phydev = NULL;
>  	phydev->attached_dev = NULL;
> -	phy_suspend(phydev);
>  	phydev->phylink = NULL;
>  
>  	phy_led_triggers_unregister(phydev);
> @@ -1171,9 +1178,12 @@ int phy_suspend(struct phy_device *phydev)
>  	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
>  	int ret = 0;
>  
> +	if (phydev->suspended)
> +		return 0;
> +
>  	/* If the device has WOL enabled, we cannot suspend the PHY */
>  	phy_ethtool_get_wol(phydev, &wol);
> -	if (wol.wolopts)
> +	if (wol.wolopts || !phy_may_suspend(phydev))
>  		return -EBUSY;
>  
>  	if (phydev->drv && phydrv->suspend)
> 

^ permalink raw reply

* [PATCH net v2] net: phy: fix WoL handling when suspending the PHY
From: Heiner Kallweit @ 2018-09-23 13:38 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org

Actually there's nothing wrong with the two changes marked as "Fixes",
they just revealed a problem which has been existing before.
After having switched r8169 to phylib it was reported that WoL from
shutdown doesn't work any longer (WoL from suspend isn't affected).
Reason is that during shutdown phy_disconnect()->phy_detach()->
phy_suspend() is called.
A similar issue occurs when the phylib state machine calls
phy_suspend() when handling state PHY_HALTED.

Core of the problem is that phy_suspend() suspends the PHY when it
should not due to WoL. phy_suspend() checks for WoL already, but this
works only if the PHY driver handles WoL (what is rarely the case).
Typically WoL is handled by the MAC driver.

phylib knows about this and handles it in mdio_bus_phy_may_suspend(),
but that's used only when suspending the system, not in other cases
like shutdown.

Therefore factor out the relevant check from
mdio_bus_phy_may_suspend() to a new function phy_may_suspend() and
use it in phy_suspend().

Last but not least change phy_detach() to call phy_suspend() before
attached_dev is set to NULL. phy_suspend() accesses attached_dev
when checking whether the MAC driver activated WoL.

Fixes: f1e911d5d0df ("r8169: add basic phylib support")
Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- improved commit message
- reduced scope of patch, don't touch functionality of
  mdio_bus_phy_suspend and mdio_bus_phy_resume
---
 drivers/net/phy/phy_device.c | 42 ++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index af64a9320..4cab94bae 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -75,6 +75,26 @@ extern struct phy_driver genphy_10g_driver;
 static LIST_HEAD(phy_fixup_list);
 static DEFINE_MUTEX(phy_fixup_lock);
 
+static bool phy_may_suspend(struct phy_device *phydev)
+{
+	struct net_device *netdev = phydev->attached_dev;
+
+	if (!netdev)
+		return true;
+
+	/* Don't suspend PHY if the attached netdev parent may wakeup.
+	 * The parent may point to a PCI device, as in tg3 driver.
+	 */
+	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
+		return false;
+
+	/* Also don't suspend PHY if the netdev itself may wakeup. This
+	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
+	 * e.g. SoC devices.
+	 */
+	return !device_may_wakeup(&netdev->dev);
+}
+
 #ifdef CONFIG_PM
 static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 {
@@ -93,20 +113,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 	if (!netdev)
 		return !phydev->suspended;
 
-	/* Don't suspend PHY if the attached netdev parent may wakeup.
-	 * The parent may point to a PCI device, as in tg3 driver.
-	 */
-	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
-		return false;
-
-	/* Also don't suspend PHY if the netdev itself may wakeup. This
-	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
-	 * e.g. SoC devices.
-	 */
-	if (device_may_wakeup(&netdev->dev))
-		return false;
-
-	return true;
+	return phy_may_suspend(phydev);
 }
 
 static int mdio_bus_phy_suspend(struct device *dev)
@@ -1132,9 +1139,9 @@ void phy_detach(struct phy_device *phydev)
 		sysfs_remove_link(&dev->dev.kobj, "phydev");
 		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
 	}
+	phy_suspend(phydev);
 	phydev->attached_dev->phydev = NULL;
 	phydev->attached_dev = NULL;
-	phy_suspend(phydev);
 	phydev->phylink = NULL;
 
 	phy_led_triggers_unregister(phydev);
@@ -1171,9 +1178,12 @@ int phy_suspend(struct phy_device *phydev)
 	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
 	int ret = 0;
 
+	if (phydev->suspended)
+		return 0;
+
 	/* If the device has WOL enabled, we cannot suspend the PHY */
 	phy_ethtool_get_wol(phydev, &wol);
-	if (wol.wolopts)
+	if (wol.wolopts || !phy_may_suspend(phydev))
 		return -EBUSY;
 
 	if (phydev->drv && phydrv->suspend)
-- 
2.19.0

^ permalink raw reply related

* [PATCH net v2] net: phy: fix WoL handling when suspending the PHY
From: Heiner Kallweit @ 2018-09-23 13:33 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org
In-Reply-To: <4dbcdd2c-96c2-2921-5016-affc8fce1d19@gmail.com>

Actually there's nothing wrong with the two changes marked as "Fixes",
they just revealed a problem which has been existing before.
After having switched r8169 to phylib it was reported that WoL from
shutdown doesn't work any longer (WoL from suspend isn't affected).
Reason is that during shutdown phy_disconnect()->phy_detach()->
phy_suspend() is called.
A similar issue occurs when the phylib state machine calls
phy_suspend() when handling state PHY_HALTED.

Core of the problem is that phy_suspend() suspends the PHY when it
should not due to WoL. phy_suspend() checks for WoL already, but this
works only if the PHY driver handles WoL (what is rarely the case).
Typically WoL is handled by the MAC driver.

phylib knows about this and handles it in mdio_bus_phy_may_suspend(),
but that's used only when suspending the system, not in other cases
like shutdown.

Therefore factor out the relevant check from
mdio_bus_phy_may_suspend() to a new function phy_may_suspend() and
use it in phy_suspend().

Last but not least change phy_detach() to call phy_suspend() before
attached_dev is set to NULL. phy_suspend() accesses attached_dev
when checking whether the MAC driver activated WoL.

Fixes: f1e911d5d0df ("r8169: add basic phylib support")
Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- improved commit message
- reduced scope of patch, don't touch functionality of
  mdio_bus_phy_suspend and mdio_bus_phy_resume
---
 drivers/net/phy/phy_device.c | 42 ++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index af64a9320..4cab94bae 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -75,6 +75,26 @@ extern struct phy_driver genphy_10g_driver;
 static LIST_HEAD(phy_fixup_list);
 static DEFINE_MUTEX(phy_fixup_lock);
 
+static bool phy_may_suspend(struct phy_device *phydev)
+{
+	struct net_device *netdev = phydev->attached_dev;
+
+	if (!netdev)
+		return true;
+
+	/* Don't suspend PHY if the attached netdev parent may wakeup.
+	 * The parent may point to a PCI device, as in tg3 driver.
+	 */
+	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
+		return false;
+
+	/* Also don't suspend PHY if the netdev itself may wakeup. This
+	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
+	 * e.g. SoC devices.
+	 */
+	return !device_may_wakeup(&netdev->dev);
+}
+
 #ifdef CONFIG_PM
 static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 {
@@ -93,20 +113,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 	if (!netdev)
 		return !phydev->suspended;
 
-	/* Don't suspend PHY if the attached netdev parent may wakeup.
-	 * The parent may point to a PCI device, as in tg3 driver.
-	 */
-	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
-		return false;
-
-	/* Also don't suspend PHY if the netdev itself may wakeup. This
-	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
-	 * e.g. SoC devices.
-	 */
-	if (device_may_wakeup(&netdev->dev))
-		return false;
-
-	return true;
+	return phy_may_suspend(phydev);
 }
 
 static int mdio_bus_phy_suspend(struct device *dev)
@@ -1132,9 +1139,9 @@ void phy_detach(struct phy_device *phydev)
 		sysfs_remove_link(&dev->dev.kobj, "phydev");
 		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
 	}
+	phy_suspend(phydev);
 	phydev->attached_dev->phydev = NULL;
 	phydev->attached_dev = NULL;
-	phy_suspend(phydev);
 	phydev->phylink = NULL;
 
 	phy_led_triggers_unregister(phydev);
@@ -1171,9 +1178,12 @@ int phy_suspend(struct phy_device *phydev)
 	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
 	int ret = 0;
 
+	if (phydev->suspended)
+		return 0;
+
 	/* If the device has WOL enabled, we cannot suspend the PHY */
 	phy_ethtool_get_wol(phydev, &wol);
-	if (wol.wolopts)
+	if (wol.wolopts || !phy_may_suspend(phydev))
 		return -EBUSY;
 
 	if (phydev->drv && phydrv->suspend)
-- 
2.19.0

^ permalink raw reply related

* Re: [PATCH] RDS: IB: Use DEFINE_PER_CPU_SHARED_ALIGNED for rds_ib_stats
From: David Miller @ 2018-09-23 19:26 UTC (permalink / raw)
  To: natechancellor
  Cc: santosh.shilimkar, netdev, linux-rdma, linux-kernel, ndesaulniers
In-Reply-To: <20180923.121927.436285462706036460.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Sun, 23 Sep 2018 12:19:27 -0700 (PDT)

> From: Nathan Chancellor <natechancellor@gmail.com>
> Date: Sat, 22 Sep 2018 23:44:45 -0700
> 
>> I have sent a v2 because this should be DECLARE_PER_CPU_SHARED_ALIGNED,
>> not DEFINE (thanks to 0day for catching it).
> 
> Your first version was already applied to the networking GIT tree, so
> you will need to submit a relative fixup with a proper "Fixes: " tag.

Nevermind, I took care of it myself.

====================
>From 16fdf8ba98391650ce4bc4f3f71629d8a413bc21 Mon Sep 17 00:00:00 2001
From: "David S. Miller" <davem@davemloft.net>
Date: Sun, 23 Sep 2018 12:25:15 -0700
Subject: [PATCH] rds: Fix build regression.

Use DECLARE_* not DEFINE_*

Fixes: 8360ed6745df ("RDS: IB: Use DEFINE_PER_CPU_SHARED_ALIGNED for rds_ib_stats")
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/rds/ib.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rds/ib.h b/net/rds/ib.h
index fd483760c910..71ff356ee702 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -443,7 +443,7 @@ int rds_ib_send_grab_credits(struct rds_ib_connection *ic, u32 wanted,
 int rds_ib_xmit_atomic(struct rds_connection *conn, struct rm_atomic_op *op);
 
 /* ib_stats.c */
-DEFINE_PER_CPU_SHARED_ALIGNED(struct rds_ib_statistics, rds_ib_stats);
+DECLARE_PER_CPU_SHARED_ALIGNED(struct rds_ib_statistics, rds_ib_stats);
 #define rds_ib_stats_inc(member) rds_stats_inc_which(rds_ib_stats, member)
 #define rds_ib_stats_add(member, count) \
 		rds_stats_add_which(rds_ib_stats, member, count)
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH] RDS: IB: Use DEFINE_PER_CPU_SHARED_ALIGNED for rds_ib_stats
From: David Miller @ 2018-09-23 19:19 UTC (permalink / raw)
  To: natechancellor
  Cc: santosh.shilimkar, netdev, linux-rdma, linux-kernel, ndesaulniers
In-Reply-To: <20180923064445.GC12338@flashbox>

From: Nathan Chancellor <natechancellor@gmail.com>
Date: Sat, 22 Sep 2018 23:44:45 -0700

> I have sent a v2 because this should be DECLARE_PER_CPU_SHARED_ALIGNED,
> not DEFINE (thanks to 0day for catching it).

Your first version was already applied to the networking GIT tree, so
you will need to submit a relative fixup with a proper "Fixes: " tag.

^ permalink raw reply

* [PATCH v3 2/2] netfilter: nf_tables: add requirements for connsecmark support
From: Christian Göttsche @ 2018-09-23 18:26 UTC (permalink / raw)
  To: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module
In-Reply-To: <20180923182616.11398-1-cgzones@googlemail.com>

Add ability to set the connection tracking secmark value.

Add ability to set the meta secmark value.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---

v3: fix compile error when CONFIG_NF_CONNTRACK_MARK not defined

Based on nf-next
Tested with v4.18.8

 net/netfilter/nft_ct.c   | 17 ++++++++++++++++-
 net/netfilter/nft_meta.c |  8 ++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index d74afa707..586627c36 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -279,7 +279,7 @@ static void nft_ct_set_eval(const struct nft_expr *expr,
 {
 	const struct nft_ct *priv = nft_expr_priv(expr);
 	struct sk_buff *skb = pkt->skb;
-#ifdef CONFIG_NF_CONNTRACK_MARK
+#if defined(CONFIG_NF_CONNTRACK_MARK) || defined(CONFIG_NF_CONNTRACK_SECMARK)
 	u32 value = regs->data[priv->sreg];
 #endif
 	enum ip_conntrack_info ctinfo;
@@ -298,6 +298,14 @@ static void nft_ct_set_eval(const struct nft_expr *expr,
 		}
 		break;
 #endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+	case NFT_CT_SECMARK:
+		if (ct->secmark != value) {
+			ct->secmark = value;
+			nf_conntrack_event_cache(IPCT_SECMARK, ct);
+		}
+		break;
+#endif
 #ifdef CONFIG_NF_CONNTRACK_LABELS
 	case NFT_CT_LABELS:
 		nf_connlabels_replace(ct,
@@ -564,6 +572,13 @@ static int nft_ct_set_init(const struct nft_ctx *ctx,
 			return -EINVAL;
 		len = sizeof(u32);
 		break;
+#endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+	case NFT_CT_SECMARK:
+		if (tb[NFTA_CT_DIRECTION])
+			return -EINVAL;
+		len = sizeof(u32);
+		break;
 #endif
 	default:
 		return -EOPNOTSUPP;
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index c8ac0ef4b..a6715c816 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -284,6 +284,11 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
 
 		skb->nf_trace = !!value8;
 		break;
+#ifdef CONFIG_NETWORK_SECMARK
+	case NFT_META_SECMARK:
+		skb->secmark = value;
+		break;
+#endif
 	default:
 		WARN_ON(1);
 	}
@@ -436,6 +441,9 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
 	switch (priv->key) {
 	case NFT_META_MARK:
 	case NFT_META_PRIORITY:
+#ifdef CONFIG_NETWORK_SECMARK
+	case NFT_META_SECMARK:
+#endif
 		len = sizeof(u32);
 		break;
 	case NFT_META_NFTRACE:
-- 
2.19.0

^ permalink raw reply related

* Re: [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support
From: kbuild test robot @ 2018-09-23 17:13 UTC (permalink / raw)
  To: Christian Göttsche
  Cc: kbuild-all, pablo, kadlec, fw, davem, netfilter-devel, coreteam,
	netdev, linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module
In-Reply-To: <20180923091611.19815-2-cgzones@googlemail.com>

[-- Attachment #1: Type: text/plain, Size: 3060 bytes --]

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on nf-next/master]
[also build test ERROR on v4.19-rc4 next-20180921]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Christian-G-ttsche/netfilter-nf_tables-add-SECMARK-support/20180923-213820
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: x86_64-randconfig-s2-09240020 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net/netfilter/nft_ct.c: In function 'nft_ct_set_eval':
>> net/netfilter/nft_ct.c:303:22: error: 'value' undeclared (first use in this function)
      if (ct->secmark != value) {
                         ^~~~~
   net/netfilter/nft_ct.c:303:22: note: each undeclared identifier is reported only once for each function it appears in

vim +/value +303 net/netfilter/nft_ct.c

   275	
   276	static void nft_ct_set_eval(const struct nft_expr *expr,
   277				    struct nft_regs *regs,
   278				    const struct nft_pktinfo *pkt)
   279	{
   280		const struct nft_ct *priv = nft_expr_priv(expr);
   281		struct sk_buff *skb = pkt->skb;
   282	#ifdef CONFIG_NF_CONNTRACK_MARK
   283		u32 value = regs->data[priv->sreg];
   284	#endif
   285		enum ip_conntrack_info ctinfo;
   286		struct nf_conn *ct;
   287	
   288		ct = nf_ct_get(skb, &ctinfo);
   289		if (ct == NULL || nf_ct_is_template(ct))
   290			return;
   291	
   292		switch (priv->key) {
   293	#ifdef CONFIG_NF_CONNTRACK_MARK
   294		case NFT_CT_MARK:
   295			if (ct->mark != value) {
   296				ct->mark = value;
   297				nf_conntrack_event_cache(IPCT_MARK, ct);
   298			}
   299			break;
   300	#endif
   301	#ifdef CONFIG_NF_CONNTRACK_SECMARK
   302		case NFT_CT_SECMARK:
 > 303			if (ct->secmark != value) {
   304				ct->secmark = value;
   305				nf_conntrack_event_cache(IPCT_SECMARK, ct);
   306			}
   307			break;
   308	#endif
   309	#ifdef CONFIG_NF_CONNTRACK_LABELS
   310		case NFT_CT_LABELS:
   311			nf_connlabels_replace(ct,
   312					      &regs->data[priv->sreg],
   313					      &regs->data[priv->sreg],
   314					      NF_CT_LABELS_MAX_SIZE / sizeof(u32));
   315			break;
   316	#endif
   317	#ifdef CONFIG_NF_CONNTRACK_EVENTS
   318		case NFT_CT_EVENTMASK: {
   319			struct nf_conntrack_ecache *e = nf_ct_ecache_find(ct);
   320			u32 ctmask = regs->data[priv->sreg];
   321	
   322			if (e) {
   323				if (e->ctmask != ctmask)
   324					e->ctmask = ctmask;
   325				break;
   326			}
   327	
   328			if (ctmask && !nf_ct_is_confirmed(ct))
   329				nf_ct_ecache_ext_add(ct, ctmask, 0, GFP_ATOMIC);
   330			break;
   331		}
   332	#endif
   333		default:
   334			break;
   335		}
   336	}
   337	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33232 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 1/5] net: allow binding socket in a VRF when there's an unbound socket
From: kbuild test robot @ 2018-09-23  9:58 UTC (permalink / raw)
  To: Mike Manning; +Cc: kbuild-all, netdev, Robert Shearman
In-Reply-To: <20180920085848.17721-2-mmanning@vyatta.att-mail.com>

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Hi Robert,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Mike-Manning/vrf-allow-simultaneous-service-instances-in-default-and-other-VRFs/20180923-162308
config: i386-randconfig-x0-09231642 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/net/tcp.h:36:0,
                    from net/ipv6/af_inet6.c:51:
   include/net/inet_hashtables.h: In function 'inet_sk_bound_dev_eq':
>> include/net/inet_hashtables.h:196:28: error: 'struct netns_ipv4' has no member named 'sysctl_tcp_l3mdev_accept'
      return !sdif || net->ipv4.sysctl_tcp_l3mdev_accept;
                               ^

vim +196 include/net/inet_hashtables.h

   191	
   192	static inline bool inet_sk_bound_dev_eq(struct net *net, int bound_dev_if,
   193						int dif, int sdif)
   194	{
   195		if (!bound_dev_if)
 > 196			return !sdif || net->ipv4.sysctl_tcp_l3mdev_accept;
   197		return bound_dev_if == dif || bound_dev_if == sdif;
   198	}
   199	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30848 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support
From: Florian Westphal @ 2018-09-23 15:41 UTC (permalink / raw)
  To: Christian Göttsche
  Cc: fw, pablo, kadlec, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, Paul Moore, Stephen Smalley, Eric Paris, jmorris,
	serge, selinux, linux-security-module
In-Reply-To: <CAJ2a_DfCmbFmpTngFnXzYy07gvEzN5UhAhrE8Cd8PowevrLUNw@mail.gmail.com>

Christian Göttsche <cgzones@googlemail.com> wrote:
> > Can you change this to:
> >
> > struct nft_secmark {
> >         u32 secid;
> >         char *ctx;
> > };
> 
> Does the nla_policy struct needs an update too? (regarding then .len member)
> 
> +static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
> +        [NFTA_SECMARK_CTX] = { .type = NLA_STRING, .len =
> NFT_SECMARK_CTX_MAXLEN },
> +}
> 
> NFT_SECMARK_CTX_MAXLEN might be dropped then..

Better keep it, we can always increase this later it if needed.
Given the length matches what xtables uses it should be fine.

^ permalink raw reply

* Re: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support
From: Christian Göttsche @ 2018-09-23 15:31 UTC (permalink / raw)
  To: fw
  Cc: pablo, kadlec, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, Paul Moore, Stephen Smalley, Eric Paris, jmorris,
	serge, selinux, linux-security-module
In-Reply-To: <20180923135555.7kwa3kyachwcfy24@breakpoint.cc>

> > +struct nft_secmark {
> > +     char ctx[NFT_SECMARK_CTX_MAXLEN];
> > +     int len;
> > +     u32 secid;
> > +};
>
> Can you change this to:
>
> struct nft_secmark {
>         u32 secid;
>         char *ctx;
> };

Does the nla_policy struct needs an update too? (regarding then .len member)

+static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
+        [NFTA_SECMARK_CTX] = { .type = NLA_STRING, .len =
NFT_SECMARK_CTX_MAXLEN },
+}

NFT_SECMARK_CTX_MAXLEN might be dropped then..

^ permalink raw reply

* [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support
From: Christian Göttsche @ 2018-09-23  9:16 UTC (permalink / raw)
  To: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module
In-Reply-To: <20180923091611.19815-1-cgzones@googlemail.com>

Add ability to set the connection tracking secmark value.

Add ability to set the meta secmark value.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---

Based on nf-next
Tested with v4.18.8

 net/netfilter/nft_ct.c   | 15 +++++++++++++++
 net/netfilter/nft_meta.c |  8 ++++++++
 2 files changed, 23 insertions(+)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index d74afa707..dcc451c20 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -298,6 +298,14 @@ static void nft_ct_set_eval(const struct nft_expr *expr,
 		}
 		break;
 #endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+	case NFT_CT_SECMARK:
+		if (ct->secmark != value) {
+			ct->secmark = value;
+			nf_conntrack_event_cache(IPCT_SECMARK, ct);
+		}
+		break;
+#endif
 #ifdef CONFIG_NF_CONNTRACK_LABELS
 	case NFT_CT_LABELS:
 		nf_connlabels_replace(ct,
@@ -564,6 +572,13 @@ static int nft_ct_set_init(const struct nft_ctx *ctx,
 			return -EINVAL;
 		len = sizeof(u32);
 		break;
+#endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+	case NFT_CT_SECMARK:
+		if (tb[NFTA_CT_DIRECTION])
+			return -EINVAL;
+		len = sizeof(u32);
+		break;
 #endif
 	default:
 		return -EOPNOTSUPP;
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index ac5df9508..555fcd66b 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -284,6 +284,11 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
 
 		skb->nf_trace = !!value8;
 		break;
+#ifdef CONFIG_NETWORK_SECMARK
+	case NFT_META_SECMARK:
+		skb->secmark = value;
+		break;
+#endif
 	default:
 		WARN_ON(1);
 	}
@@ -436,6 +441,9 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
 	switch (priv->key) {
 	case NFT_META_MARK:
 	case NFT_META_PRIORITY:
+#ifdef CONFIG_NETWORK_SECMARK
+	case NFT_META_SECMARK:
+#endif
 		len = sizeof(u32);
 		break;
 	case NFT_META_NFTRACE:
-- 
2.19.0

^ permalink raw reply related

* Re: [PATCH net-next 1/5] net: allow binding socket in a VRF when there's an unbound socket
From: kbuild test robot @ 2018-09-23  8:47 UTC (permalink / raw)
  To: Mike Manning; +Cc: kbuild-all, netdev, Robert Shearman
In-Reply-To: <20180920085848.17721-2-mmanning@vyatta.att-mail.com>

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

Hi Robert,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Mike-Manning/vrf-allow-simultaneous-service-instances-in-default-and-other-VRFs/20180923-162308
config: x86_64-randconfig-x004-201838 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from include/net/tcp.h:36:0,
                    from net/core/sock.c:143:
   include/net/inet_hashtables.h: In function 'inet_sk_bound_dev_eq':
>> include/net/inet_hashtables.h:196:29: error: 'struct netns_ipv4' has no member named 'sysctl_tcp_l3mdev_accept'; did you mean 'sysctl_tcp_fwmark_accept'?
      return !sdif || net->ipv4.sysctl_tcp_l3mdev_accept;
                                ^~~~~~~~~~~~~~~~~~~~~~~~
                                sysctl_tcp_fwmark_accept

vim +196 include/net/inet_hashtables.h

   191	
   192	static inline bool inet_sk_bound_dev_eq(struct net *net, int bound_dev_if,
   193						int dif, int sdif)
   194	{
   195		if (!bound_dev_if)
 > 196			return !sdif || net->ipv4.sysctl_tcp_l3mdev_accept;
   197		return bound_dev_if == dif || bound_dev_if == sdif;
   198	}
   199	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32260 bytes --]

^ permalink raw reply

* Re: net build error
From: Dmitry Vyukov @ 2018-09-23  8:14 UTC (permalink / raw)
  To: syzbot, netdev; +Cc: LKML, syzkaller-bugs
In-Reply-To: <0000000000002cdabf057685027b@google.com>

On Sun, Sep 23, 2018 at 9:42 AM, syzbot
<syzbot+a72ba31e3224309179d4@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    474ff2600889 net-ethtool: ETHTOOL_GUFO did not and should ..
> git tree:       net
> console output: https://syzkaller.appspot.com/x/log.txt?x=164c4059400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=5fa12be50bca08d8
> dashboard link: https://syzkaller.appspot.com/bug?extid=a72ba31e3224309179d4
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+a72ba31e3224309179d4@syzkaller.appspotmail.com

+netdev

The root cause:


net/rds/ib.o:(.data..percpu..shared_aligned+0x0): multiple definition
of `rds_ib_stats'
net/rds/rdma_transport.o:(.data..percpu..shared_aligned+0x0): first defined here
net/rds/ib_cm.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_recv.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_ring.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_send.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_stats.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_sysctl.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_rdma.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_fmr.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_frmr.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
Makefile:1030: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1


> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.

^ permalink raw reply

* Re: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support
From: Florian Westphal @ 2018-09-23 13:55 UTC (permalink / raw)
  To: Christian Göttsche
  Cc: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module
In-Reply-To: <20180923091611.19815-1-cgzones@googlemail.com>

Christian Göttsche <cgzones@googlemail.com> wrote:
> Add the ability to set the security context of packets within the nf_tables framework.
> Add a nft_object for holding security contexts in the kernel and manipulating packets on the wire.
> 
> Convert the security context strings at rule addition time to security identifiers.
> This is the same behavior like in xt_SECMARK and offers better performance than computing it per packet.
> 
> Set the maximum security context length to 256.

Looks good, one minor suggestion.

> +#ifdef CONFIG_NETWORK_SECMARK
> +
> +struct nft_secmark {
> +	char ctx[NFT_SECMARK_CTX_MAXLEN];
> +	int len;
> +	u32 secid;
> +};

Can you change this to:

struct nft_secmark {
	u32 secid;
	char *ctx;
};

?
We don't need ctx in the packetpath, so better to keep
the struct size small.

> +	nla_strlcpy(priv->ctx, tb[NFTA_SECMARK_CTX], NFT_SECMARK_CTX_MAXLEN);

You can change this to
	priv->ctx = nla_strdup(tb[NFTA_SECMARK_CTX], GFP_KERNEL);
	if (!priv->ctx)
		return -ENOMEM;

> +	err = nft_secmark_secconversion(priv);
> +	if (err) {
		kfree(priv->ctx);

> +static void nft_secmark_obj_destroy(const struct nft_ctx *ctx, struct nft_object *obj)
> +{
	kfree(priv->ctx);

But other than this i think this is ready to be applied,
thanks a lot for making this happen.

^ permalink raw reply

* Re: [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support
From: Florian Westphal @ 2018-09-23 13:51 UTC (permalink / raw)
  To: Christian Göttsche
  Cc: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module
In-Reply-To: <20180923091611.19815-2-cgzones@googlemail.com>

Christian Göttsche <cgzones@googlemail.com> wrote:
> Add ability to set the connection tracking secmark value.
> Add ability to set the meta secmark value.

Looks good to me.
Acked-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply

* Re: [PATCH nf-next] netfilter: ctnetlink: must check mark attributes vs NULL
From: Kristian Evensen @ 2018-09-23  7:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Florian Westphal, Netfilter Development Mailing list,
	syzbot+e45eda8eda6e93a03959, syzkaller-bugs, Network Development
In-Reply-To: <20180921081458.l42yaho6r6sfd76f@salvia>

Florian,
On Fri, Sep 21, 2018 at 10:15 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Thu, Sep 20, 2018 at 11:53:06PM +0200, Florian Westphal wrote:
> > else we will oops (null deref) when the attributes aren't present.
> >
> > Also add back the EOPNOTSUPP in case MARK filtering is requested but
> > kernel doesn't support it.
>
> Applied, thanks Florian.

Thanks for fixing my embarrassing mistake!

BR,
Kristian

^ permalink raw reply

* Re: [PATCH v2] RDS: IB: Use DECLARE_PER_CPU_SHARED_ALIGNED for rds_ib_stats
From: santosh.shilimkar @ 2018-09-23  7:39 UTC (permalink / raw)
  To: Nathan Chancellor, David S. Miller
  Cc: netdev, linux-rdma, linux-kernel, Nick Desaulniers
In-Reply-To: <20180923032938.27093-1-natechancellor@gmail.com>

On 9/22/18 8:29 PM, Nathan Chancellor wrote:
> Clang warns when two declarations' section attributes don't match.
> 
> net/rds/ib_stats.c:40:1: warning: section does not match previous
> declaration [-Wsection]
> DEFINE_PER_CPU_SHARED_ALIGNED(struct rds_ib_statistics, rds_ib_stats);
> ^
> ./include/linux/percpu-defs.h:142:2: note: expanded from macro
> 'DEFINE_PER_CPU_SHARED_ALIGNED'
>          DEFINE_PER_CPU_SECTION(type, name,
> PER_CPU_SHARED_ALIGNED_SECTION) \
>          ^
> ./include/linux/percpu-defs.h:93:9: note: expanded from macro
> 'DEFINE_PER_CPU_SECTION'
>          extern __PCPU_ATTRS(sec) __typeof__(type) name;
> \
>                 ^
> ./include/linux/percpu-defs.h:49:26: note: expanded from macro
> '__PCPU_ATTRS'
>          __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))
> \
>                                  ^
> net/rds/ib.h:446:1: note: previous attribute is here
> DECLARE_PER_CPU(struct rds_ib_statistics, rds_ib_stats);
> ^
> ./include/linux/percpu-defs.h:111:2: note: expanded from macro
> 'DECLARE_PER_CPU'
>          DECLARE_PER_CPU_SECTION(type, name, "")
>          ^
> ./include/linux/percpu-defs.h:87:9: note: expanded from macro
> 'DECLARE_PER_CPU_SECTION'
>          extern __PCPU_ATTRS(sec) __typeof__(type) name
>                 ^
> ./include/linux/percpu-defs.h:49:26: note: expanded from macro
> '__PCPU_ATTRS'
>          __percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))
> \
>                                  ^
> 1 warning generated.
> 
> The initial definition was added in commit ec16227e1414 ("RDS/IB:
> Infiniband transport") and the cache aligned definition was added in
> commit e6babe4cc4ce ("RDS/IB: Stats and sysctls") right after. The
> definition probably should have been updated in net/rds/ib.h, which is
> what this patch does.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/114
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> 
> v1 -> v2:
> 
> Fix reported build error by using DECLARE instead of DEFINE
> 
Thanks for update !!
Acked-by: Santosh shilimkar <santosh.shilimkar@oracle.com>

^ permalink raw reply

* [PATCH] iavh: fix a typo
From: Rami Rosen @ 2018-09-23  7:21 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, Rami Rosen

This trivial patch fixes a typo in iavf.h.

Signed-off-by: Rami Rosen <ramirose@gmail.com>
---
 drivers/net/ethernet/intel/iavf/iavf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index a512f7521841..272d76b733aa 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -342,7 +342,7 @@ struct iavf_adapter {
 	struct iavf_channel_config ch_config;
 	u8 num_tc;
 	struct list_head cloud_filter_list;
-	/* lock to protest access to the cloud filter list */
+	/* lock to protect access to the cloud filter list */
 	spinlock_t cloud_filter_list_lock;
 	u16 num_cloud_filters;
 };
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] rtlwifi: btcoex: Use proper enumerated types for Wi-Fi only interface
From: Nathan Chancellor @ 2018-09-23  6:43 UTC (permalink / raw)
  To: Ping-Ke Shih, Kalle Valo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20180921220824.18344-1-natechancellor@gmail.com>

On Fri, Sep 21, 2018 at 03:08:24PM -0700, Nathan Chancellor wrote:
> Clang warns when one enumerated type is implicitly converted to another.
> 
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1327:34:
> warning: implicit conversion from enumeration type 'enum
> btc_chip_interface' to different enumeration type 'enum
> wifionly_chip_interface' [-Wenum-conversion]
>                 wifionly_cfg->chip_interface = BTC_INTF_PCI;
>                                              ~ ^~~~~~~~~~~~
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1330:34:
> warning: implicit conversion from enumeration type 'enum
> btc_chip_interface' to different enumeration type 'enum
> wifionly_chip_interface' [-Wenum-conversion]
>                 wifionly_cfg->chip_interface = BTC_INTF_USB;
>                                              ~ ^~~~~~~~~~~~
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1333:34:
> warning: implicit conversion from enumeration type 'enum
> btc_chip_interface' to different enumeration type 'enum
> wifionly_chip_interface' [-Wenum-conversion]
>                 wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
>                                              ~ ^~~~~~~~~~~~~~~~
> 3 warnings generated.
> 
> Use the values from the correct enumerated type, wifionly_chip_interface.
> 
> BTC_INTF_UNKNOWN = WIFIONLY_INTF_UNKNOWN = 0
> BTC_INTF_PCI = WIFIONLY_INTF_PCI = 0
> BTC_INTF_USB = WIFIONLY_INTF_USB = 0
> 

I have sent a v2 making these values correct.

I will make sure the '--in-reply-to' option in the future.

Nathan

> Link: https://github.com/ClangBuiltLinux/linux/issues/135
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  .../net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c   | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> index b026e80940a4..6fbf8845a2ab 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> @@ -1324,13 +1324,13 @@ bool exhalbtc_initlize_variables_wifi_only(struct rtl_priv *rtlpriv)
>  
>  	switch (rtlpriv->rtlhal.interface) {
>  	case INTF_PCI:
> -		wifionly_cfg->chip_interface = BTC_INTF_PCI;
> +		wifionly_cfg->chip_interface = WIFIONLY_INTF_PCI;
>  		break;
>  	case INTF_USB:
> -		wifionly_cfg->chip_interface = BTC_INTF_USB;
> +		wifionly_cfg->chip_interface = WIFIONLY_INTF_USB;
>  		break;
>  	default:
> -		wifionly_cfg->chip_interface = BTC_INTF_UNKNOWN;
> +		wifionly_cfg->chip_interface = WIFIONLY_INTF_UNKNOWN;
>  		break;
>  	}
>  
> -- 
> 2.19.0
> 

^ permalink raw reply

* Re: [PATCH v7 4/4] gpiolib: Implement fast processing path in get/set array
From: Janusz Krzysztofik @ 2018-09-23 10:43 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Andrew Lunn, Ulf Hansson, linux-doc, linux-iio, Linus Walleij,
	Dominik Brodowski, Peter Rosin, netdev, linux-i2c,
	Peter Meerwald-Stadler, devel, Florian Fainelli, Jonathan Corbet,
	Janusz Krzysztofik, Krzysztof Kozlowski, Kishon Vijay Abraham I,
	Tony Lindgren, Lukas Wunner, Geert Uytterhoeven, linux-serial,
	Jiri Slaby, Michael Hennerich, Uwe Kleine-König, linux-gpio
In-Reply-To: <20180921141409eucas1p190a47e2608429870d23516ee5e75c191~Wb8z_FmwI1466414664eucas1p1F@eucas1p1.samsung.com>

On Friday, September 21, 2018 4:14:06 PM CEST Marek Szyprowski wrote:
> Hi Janusz,
> 
> 
> On 2018-09-21 12:51, Janusz Krzysztofik wrote:
> > 2018-09-21 10:18 GMT+02:00, Marek Szyprowski <m.szyprowski@samsung.com>:
> >> On 2018-09-20 18:21, Janusz Krzysztofik wrote:
> >>> On Thursday, September 20, 2018 5:48:22 PM CEST Janusz Krzysztofik wrote:
> >>>> On Thursday, September 20, 2018 12:11:48 PM CEST Marek Szyprowski 
wrote:
> >>>>> On 2018-09-02 14:01, Janusz Krzysztofik wrote:
> >>>>>> Certain GPIO descriptor arrays returned by gpio_get_array() may
> >>>>>> contain
> >>>>>> information on direct mapping of array members to pins of a single
> >>>>>> GPIO
> >>>>>> chip in hardware order.  In such cases, bitmaps of values can be
> >>>>>> passed
> >>>>>> directly from/to the chip's .get/set_multiple() callbacks without
> >>>>>> wasting time on iterations.
> >>>>>>
> >>>>>> Add respective code to gpiod_get/set_array_bitmap_complex() 
functions.
> >>>>>> Pins not applicable for fast path are processed as before, skipping
> >>>>>> over the 'fast' ones.
> >>>>>>
> >>>>>> Cc: Jonathan Corbet <corbet@lwn.net>
> >>>>>> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> >>>>> I've just noticed that this patch landed in today's linux-next. Sadly
> >>>>> it
> >>>>> breaks booting of Exynos5250-based Samsung Snow Chromebook (ARM 32bit,
> >>>>> device-tree source arch/arm/boot/dts/exynos5250-snow.dts).
> >>>>>
> >>>>> Booting hangs after detecting MMC cards. Reverting this patch fixes the
> >>>>> boot. I will try later to add some debugs and investigate it further
> >>>>> what
> >>>>> really happens when booting hangs.
> >>>> Hi Marek,
> >>>>
> >>>> Thanks for reporting. Could you please try the following fix?
> >>> Hi again,
> >>>
> >>> I realized the patch was not correct, j, not i, should be updated in
> >>> second
> >>> hunk. Please try the following one.
> >>>
> >>> Thanks,
> >>> Janusz
> >>>
> >>> >From a919c504850f6cb40e8e81267a3a37537f7c4fd4 Mon Sep 17 00:00:00 2001
> >>> From: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> >>> Date: Thu, 20 Sep 2018 17:37:21 +0200
> >>> Subject: [PATCH] gpiolib: Fix bitmap index not updated
> >>> While skipping fast path bits, bitmap index is not updated with next
> >>> found zero bit position. Fix it.
> >>>
> >>> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> >> This one also doesn't help. A quick compare of logs with this version and
> >> a working system shows, that with your patch (and fix) there are no calls
> >> to
> >> gpx0-2 pin (which are a part of mmc pwrseq), what causes mmc failure. If
> >> you need any more information (what kind of logs will help?), let me 
know.
> > There is a debug message on array_info content available at the end of
> > gpiod_get_array(), could you please activate it and post the message so
> > we can understand better what is going on?
> 
> With debug enabled on next-20180919:
> [    2.499153] pwrseq_simple mmc3_pwrseq: GPIO array info: chip=gpx0, 
> size=2, get_mask=2, set_mask=2, invert_mask=2

Looks good to me, i..e., in line with what one could expect.  However, ...

> On next-20180920 I get no this message and booting hangs.
> 
> Same with next-20180920 + your second fix from this thread.
> 
> I will try to debug this more on Monday.
> 
> > On the other hand, I've had a look your device-tree configuration and
> > it looks like that specific setup won't benefit from the fast bitmap path.
> > You have pin 2 at position 0 and pin 1 at position 1 of the array.
> > Hence, the fast bitmap path covers only pin 1, and pin 2 is processed
> > by the old path with apparently buggy code for skipping over fast pins.
> >
> > As a temporary workaround, you could try to revert the order of pins in
> > your dts file (pin 1 at position 0, pin 2 at 1) and the mmc pwrseq code
> > should work for you again by taking the original old path, not skipping
> > over fast pins.  Results of such check may also help us to better
> > understand and resolve the issue.
> 
> Changing the order of mmc pwrseq gpio pins fixes boot hang.

Not being able to discover more coding bugs in the code modified by the series, 
I'm wondering if the reason for the issue you are observing comes from the 
fact both pins are no longer manipulated together within a single 
.set_multiple() chip callback. I'm working on a fix which prevents from that.

Thanks,
Janusz

^ permalink raw reply

* [PATCH net-next 2/2] neighbour: send netlink notification if NTF_ROUTER changes
From: Roopa Prabhu @ 2018-09-23  4:26 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1537676780-5370-1-git-send-email-roopa@cumulusnetworks.com>

From: Roopa Prabhu <roopa@cumulusnetworks.com>

send netlink notification if neigh_update results in NTF_ROUTER
change and if NEIGH_UPDATE_F_ISROUTER is on. Also move the
NTF_ROUTER change function into a helper.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 include/net/neighbour.h | 15 +++++++++++++++
 net/core/neighbour.c    |  7 ++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 6c1eecd..0874f7fc 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -544,4 +544,19 @@ static inline void neigh_update_ext_learned(struct neighbour *neigh, u32 flags,
 		*notify = 1;
 	}
 }
+
+static inline void neigh_update_is_router(struct neighbour *neigh, u32 flags,
+					  int *notify)
+{
+	u8 ndm_flags = 0;
+
+	ndm_flags |= (flags & NEIGH_UPDATE_F_ISROUTER) ? NTF_ROUTER : 0;
+	if ((neigh->flags ^ ndm_flags) & NTF_ROUTER) {
+		if (ndm_flags & NTF_ROUTER)
+			neigh->flags |= NTF_ROUTER;
+		else
+			neigh->flags &= ~NTF_ROUTER;
+		*notify = 1;
+	}
+}
 #endif
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index ca99456..fb89294 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1277,11 +1277,8 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 		neigh->arp_queue_len_bytes = 0;
 	}
 out:
-	if (update_isrouter) {
-		neigh->flags = (flags & NEIGH_UPDATE_F_ISROUTER) ?
-			(neigh->flags | NTF_ROUTER) :
-			(neigh->flags & ~NTF_ROUTER);
-	}
+	if (update_isrouter)
+		neigh_update_is_router(neigh, flags, &notify);
 	write_unlock_bh(&neigh->lock);
 
 	if (notify)
-- 
2.1.4

^ permalink raw reply related

* [PATCH net-next 1/2] neighbour: allow admin to set NTF_ROUTER
From: Roopa Prabhu @ 2018-09-23  4:26 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <1537676780-5370-1-git-send-email-roopa@cumulusnetworks.com>

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch allows admin setting of NTF_ROUTER flag
on a neighbour entry. This enables external control
plane (like bgp evpn) to manage neigh entries with
NTF_ROUTER flag.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 net/core/neighbour.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index aa19d86..ca99456 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1709,7 +1709,8 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh,
 static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 		     struct netlink_ext_ack *extack)
 {
-	int flags = NEIGH_UPDATE_F_ADMIN | NEIGH_UPDATE_F_OVERRIDE;
+	int flags = NEIGH_UPDATE_F_ADMIN | NEIGH_UPDATE_F_OVERRIDE |
+		NEIGH_UPDATE_F_OVERRIDE_ISROUTER;
 	struct net *net = sock_net(skb->sk);
 	struct ndmsg *ndm;
 	struct nlattr *tb[NDA_MAX+1];
@@ -1784,12 +1785,16 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 		}
 
 		if (!(nlh->nlmsg_flags & NLM_F_REPLACE))
-			flags &= ~NEIGH_UPDATE_F_OVERRIDE;
+			flags &= ~(NEIGH_UPDATE_F_OVERRIDE |
+				   NEIGH_UPDATE_F_OVERRIDE_ISROUTER);
 	}
 
 	if (ndm->ndm_flags & NTF_EXT_LEARNED)
 		flags |= NEIGH_UPDATE_F_EXT_LEARNED;
 
+	if (ndm->ndm_flags & NTF_ROUTER)
+		flags |= NEIGH_UPDATE_F_ISROUTER;
+
 	if (ndm->ndm_flags & NTF_USE) {
 		neigh_event_send(neigh, NULL);
 		err = 0;
-- 
2.1.4

^ permalink raw reply related

* [PATCH net-next 0/2] few NTF_ROUTER related updates
From: Roopa Prabhu @ 2018-09-23  4:26 UTC (permalink / raw)
  To: davem; +Cc: netdev

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This series allows setting of NTF_ROUTER by an external
entity (eg BGP E-VPN control plane). Also fixes missing
netlink notification on neigh NTF_ROUTER flag changes.

Roopa Prabhu (2):
  neighbour: allow admin to set NTF_ROUTER
  neighbour: send netlink notification if NTF_ROUTER changes

 include/net/neighbour.h | 15 +++++++++++++++
 net/core/neighbour.c    | 16 +++++++++-------
 2 files changed, 24 insertions(+), 7 deletions(-)

-- 
2.1.4

^ permalink raw reply

* [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support
From: Christian Göttsche @ 2018-09-23  9:16 UTC (permalink / raw)
  To: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module

Add the ability to set the security context of packets within the nf_tables framework.
Add a nft_object for holding security contexts in the kernel and manipulating packets on the wire.

Convert the security context strings at rule addition time to security identifiers.
This is the same behavior like in xt_SECMARK and offers better performance than computing it per packet.

Set the maximum security context length to 256.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---

v2: convert security context strings to ids on rule addition time

Based on nf-next
Tested with v4.18.8

 include/net/netfilter/nf_tables_core.h   |   4 +
 include/uapi/linux/netfilter/nf_tables.h |  18 +++-
 net/netfilter/nf_tables_core.c           |  28 ++++++-
 net/netfilter/nft_meta.c                 | 101 +++++++++++++++++++++++
 4 files changed, 146 insertions(+), 5 deletions(-)

diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
index 8da837d2a..2046d104f 100644
--- a/include/net/netfilter/nf_tables_core.h
+++ b/include/net/netfilter/nf_tables_core.h
@@ -16,6 +16,10 @@ extern struct nft_expr_type nft_meta_type;
 extern struct nft_expr_type nft_rt_type;
 extern struct nft_expr_type nft_exthdr_type;
 
+#ifdef CONFIG_NETWORK_SECMARK
+extern struct nft_object_type nft_secmark_obj_type;
+#endif
+
 int nf_tables_core_module_init(void);
 void nf_tables_core_module_exit(void);
 
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 702e4f0be..5444e7687 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1176,6 +1176,21 @@ enum nft_quota_attributes {
 };
 #define NFTA_QUOTA_MAX		(__NFTA_QUOTA_MAX - 1)
 
+/**
+ * enum nft_secmark_attributes - nf_tables secmark object netlink attributes
+ *
+ * @NFTA_SECMARK_CTX: security context (NLA_STRING)
+ */
+enum nft_secmark_attributes {
+	NFTA_SECMARK_UNSPEC,
+	NFTA_SECMARK_CTX,
+	__NFTA_SECMARK_MAX,
+};
+#define NFTA_SECMARK_MAX	(__NFTA_SECMARK_MAX - 1)
+
+/* Max security context length */
+#define NFT_SECMARK_CTX_MAXLEN		256
+
 /**
  * enum nft_reject_types - nf_tables reject expression reject types
  *
@@ -1432,7 +1447,8 @@ enum nft_ct_timeout_timeout_attributes {
 #define NFT_OBJECT_CONNLIMIT	5
 #define NFT_OBJECT_TUNNEL	6
 #define NFT_OBJECT_CT_TIMEOUT	7
-#define __NFT_OBJECT_MAX	8
+#define NFT_OBJECT_SECMARK	8
+#define __NFT_OBJECT_MAX	9
 #define NFT_OBJECT_MAX		(__NFT_OBJECT_MAX - 1)
 
 /**
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index ffd5c0f94..3fbce3b9c 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -249,12 +249,24 @@ static struct nft_expr_type *nft_basic_types[] = {
 	&nft_exthdr_type,
 };
 
+static struct nft_object_type *nft_basic_objects[] = {
+#ifdef CONFIG_NETWORK_SECMARK
+	&nft_secmark_obj_type,
+#endif
+};
+
 int __init nf_tables_core_module_init(void)
 {
-	int err, i;
+	int err, i, j = 0;
+
+	for (i = 0; i < ARRAY_SIZE(nft_basic_objects); i++) {
+		err = nft_register_obj(nft_basic_objects[i]);
+		if (err)
+			goto err;
+	}
 
-	for (i = 0; i < ARRAY_SIZE(nft_basic_types); i++) {
-		err = nft_register_expr(nft_basic_types[i]);
+	for (j = 0; j < ARRAY_SIZE(nft_basic_types); j++) {
+		err = nft_register_expr(nft_basic_types[j]);
 		if (err)
 			goto err;
 	}
@@ -262,8 +274,12 @@ int __init nf_tables_core_module_init(void)
 	return 0;
 
 err:
+	while (j-- > 0)
+		nft_unregister_expr(nft_basic_types[j]);
+
 	while (i-- > 0)
-		nft_unregister_expr(nft_basic_types[i]);
+		nft_unregister_obj(nft_basic_objects[i]);
+
 	return err;
 }
 
@@ -274,4 +290,8 @@ void nf_tables_core_module_exit(void)
 	i = ARRAY_SIZE(nft_basic_types);
 	while (i-- > 0)
 		nft_unregister_expr(nft_basic_types[i]);
+
+	i = ARRAY_SIZE(nft_basic_objects);
+	while (i-- > 0)
+		nft_unregister_obj(nft_basic_objects[i]);
 }
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 297fe7d97..ac5df9508 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -543,3 +543,104 @@ struct nft_expr_type nft_meta_type __read_mostly = {
 	.maxattr	= NFTA_META_MAX,
 	.owner		= THIS_MODULE,
 };
+
+#ifdef CONFIG_NETWORK_SECMARK
+
+struct nft_secmark {
+	char ctx[NFT_SECMARK_CTX_MAXLEN];
+	int len;
+	u32 secid;
+};
+
+static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
+	[NFTA_SECMARK_CTX]     = { .type = NLA_STRING, .len = NFT_SECMARK_CTX_MAXLEN },
+};
+
+static int nft_secmark_secconversion(struct nft_secmark *priv)
+{
+	int err;
+	u32 tmp_secid = 0;
+
+	err = security_secctx_to_secid(priv->ctx, priv->len, &tmp_secid);
+	if (err)
+		return err;
+
+	if (!tmp_secid)
+		return -ENOENT;
+
+	err = security_secmark_relabel_packet(tmp_secid);
+	if (err)
+		return err;
+
+	priv->secid = tmp_secid;
+	return 0;
+}
+
+static void nft_secmark_obj_eval(struct nft_object *obj, struct nft_regs *regs, const struct nft_pktinfo *pkt)
+{
+	const struct nft_secmark *priv = nft_obj_data(obj);
+	struct sk_buff *skb = pkt->skb;
+
+	skb->secmark = priv->secid;
+}
+
+
+static int nft_secmark_obj_init(const struct nft_ctx *ctx, const struct nlattr * const tb[], struct nft_object *obj)
+{
+	int err;
+	struct nft_secmark *priv = nft_obj_data(obj);
+
+	if (tb[NFTA_SECMARK_CTX] == NULL)
+		return -EINVAL;
+
+	nla_strlcpy(priv->ctx, tb[NFTA_SECMARK_CTX], NFT_SECMARK_CTX_MAXLEN);
+	priv->len = strlen(priv->ctx);
+
+	err = nft_secmark_secconversion(priv);
+	if (err)
+		return err;
+
+	security_secmark_refcount_inc();
+
+	return 0;
+}
+
+static int nft_secmark_obj_dump(struct sk_buff *skb, struct nft_object *obj, bool reset)
+{
+	int err;
+	struct nft_secmark *priv = nft_obj_data(obj);
+
+	if (nla_put_string(skb, NFTA_SECMARK_CTX, priv->ctx))
+		return -1;
+
+	if (reset) {
+		err = nft_secmark_secconversion(priv);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static void nft_secmark_obj_destroy(const struct nft_ctx *ctx, struct nft_object *obj)
+{
+	security_secmark_refcount_dec();
+}
+
+static const struct nft_object_ops nft_secmark_obj_ops = {
+	.type		= &nft_secmark_obj_type,
+	.size		= sizeof(struct nft_secmark),
+	.init		= nft_secmark_obj_init,
+	.eval		= nft_secmark_obj_eval,
+	.dump		= nft_secmark_obj_dump,
+	.destroy	= nft_secmark_obj_destroy,
+};
+struct nft_object_type nft_secmark_obj_type __read_mostly = {
+	.type		= NFT_OBJECT_SECMARK,
+	.ops		= &nft_secmark_obj_ops,
+	.maxattr	= NFTA_SECMARK_MAX,
+	.policy		= nft_secmark_policy,
+	.owner		= THIS_MODULE,
+};
+
+#endif /* CONFIG_NETWORK_SECMARK */
-- 
2.19.0

^ permalink raw reply related

* Re: [PATCH] net/mlx4: Use cpumask_available for eq->affinity_mask
From: Tariq Toukan @ 2018-09-23  7:52 UTC (permalink / raw)
  To: Nathan Chancellor, Tariq Toukan, David S. Miller
  Cc: netdev, linux-rdma, linux-kernel
In-Reply-To: <20180921094412.15547-1-natechancellor@gmail.com>



On 21/09/2018 12:44 PM, Nathan Chancellor wrote:
> Clang warns that the address of a pointer will always evaluated as true
> in a boolean context:
> 
> drivers/net/ethernet/mellanox/mlx4/eq.c:243:11: warning: address of
> array 'eq->affinity_mask' will always evaluate to 'true'
> [-Wpointer-bool-conversion]
>          if (!eq->affinity_mask || cpumask_empty(eq->affinity_mask))
>              ~~~~~^~~~~~~~~~~~~
> 1 warning generated.
> 
> Use cpumask_available, introduced in commit f7e30f01a9e2 ("cpumask: Add
> helper cpumask_available()"), which does the proper checking and avoids
> this warning.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/86
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>   drivers/net/ethernet/mellanox/mlx4/eq.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
> index 1f3372c1802e..2df92dbd38e1 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/eq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
> @@ -240,7 +240,8 @@ static void mlx4_set_eq_affinity_hint(struct mlx4_priv *priv, int vec)
>   	struct mlx4_dev *dev = &priv->dev;
>   	struct mlx4_eq *eq = &priv->eq_table.eq[vec];
>   
> -	if (!eq->affinity_mask || cpumask_empty(eq->affinity_mask))
> +	if (!cpumask_available(eq->affinity_mask) ||
> +	    cpumask_empty(eq->affinity_mask))
>   		return;
>   
>   	hint_err = irq_set_affinity_hint(eq->irq, eq->affinity_mask);
> 

Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox