Netdev List
 help / color / mirror / Atom feed
* [PATCH 1/3] netfilter: nf_tables: fix chain filter in nf_tables_dump_rules()
From: Pablo Neira Ayuso @ 2018-01-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180105150825.18460-1-pablo@netfilter.org>

ctx->chain may be null now that we have very large object names,
so we cannot check for ctx->chain[0] here.

Fixes: b7263e071aba7 ("netfilter: nf_tables: Allow table names of up to 255 chars")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Phil Sutter <phil@nwl.cc>
---
 net/netfilter/nf_tables_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 10798b357481..8d4526651661 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2072,7 +2072,7 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
 				continue;
 
 			list_for_each_entry_rcu(chain, &table->chains, list) {
-				if (ctx && ctx->chain[0] &&
+				if (ctx && ctx->chain &&
 				    strcmp(ctx->chain, chain->name) != 0)
 					continue;
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH 2/3] netfilter: uapi: correct UNTRACKED conntrack state bit number
From: Pablo Neira Ayuso @ 2018-01-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180105150825.18460-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

nft_ct exposes this bit to userspace.  This used to be

  #define NF_CT_STATE_UNTRACKED_BIT              (1 << (IP_CT_NUMBER + 1))
  (IP_CT_NUMBER is 5, so this was 0x40)

.. but this got changed to 8 (0x100) when the untracked object got removed.
Replace this with a literal 6 to prevent further incompatible changes
in case IP_CT_NUMBER ever increases.

Fixes: cc41c84b7e7f2 ("netfilter: kill the fake untracked conntrack objects")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter/nf_conntrack_common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h b/include/uapi/linux/netfilter/nf_conntrack_common.h
index 3fea7709a441..57ccfb32e87f 100644
--- a/include/uapi/linux/netfilter/nf_conntrack_common.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_common.h
@@ -36,7 +36,7 @@ enum ip_conntrack_info {
 
 #define NF_CT_STATE_INVALID_BIT			(1 << 0)
 #define NF_CT_STATE_BIT(ctinfo)			(1 << ((ctinfo) % IP_CT_IS_REPLY + 1))
-#define NF_CT_STATE_UNTRACKED_BIT		(1 << (IP_CT_UNTRACKED + 1))
+#define NF_CT_STATE_UNTRACKED_BIT		(1 << 6)
 
 /* Bitset representing status of connection. */
 enum ip_conntrack_status {
-- 
2.11.0


^ permalink raw reply related

* [PATCH 3/3] netfilter: nf_tables: fix potential NULL-ptr deref in nf_tables_dump_obj_done()
From: Pablo Neira Ayuso @ 2018-01-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180105150825.18460-1-pablo@netfilter.org>

From: Hangbin Liu <liuhangbin@gmail.com>

If there is no NFTA_OBJ_TABLE and NFTA_OBJ_TYPE, the c.data will be NULL in
nf_tables_getobj(). So before free filter->table in nf_tables_dump_obj_done(),
we need to check if filter is NULL first.

Fixes: e46abbcc05aa ("netfilter: nf_tables: Allow table names of up to 255 chars")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Acked-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8d4526651661..07bd4138c84e 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -4665,8 +4665,10 @@ static int nf_tables_dump_obj_done(struct netlink_callback *cb)
 {
 	struct nft_obj_filter *filter = cb->data;
 
-	kfree(filter->table);
-	kfree(filter);
+	if (filter) {
+		kfree(filter->table);
+		kfree(filter);
+	}
 
 	return 0;
 }
-- 
2.11.0


^ permalink raw reply related

* [PATCH 0/3] Netfilter fixes for net
From: Pablo Neira Ayuso @ 2018-01-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following patchset contains Netfilter fixes for your net tree,
they are:

1) Fix chain filtering when dumping rules via nf_tables_dump_rules().

2) Fix accidental change in NF_CT_STATE_UNTRACKED_BIT through uapi,
   introduced when removing the untracked conntrack object, from
   Florian Westphal.

3) Fix potential nul-dereference when releasing dump filter in
   nf_tables_dump_obj_done(), patch from Hangbin Liu.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Thanks!

----------------------------------------------------------------

The following changes since commit b4681c2829e24943aadd1a7bb3a30d41d0a20050:

  ipv4: Fix use-after-free when flushing FIB tables (2017-12-20 15:12:39 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD

for you to fetch changes up to 8bea728dce8972e534e6b99fd550f7b5cc3864e8:

  netfilter: nf_tables: fix potential NULL-ptr deref in nf_tables_dump_obj_done() (2017-12-26 17:16:47 +0100)

----------------------------------------------------------------
Florian Westphal (1):
      netfilter: uapi: correct UNTRACKED conntrack state bit number

Hangbin Liu (1):
      netfilter: nf_tables: fix potential NULL-ptr deref in nf_tables_dump_obj_done()

Pablo Neira Ayuso (1):
      netfilter: nf_tables: fix chain filter in nf_tables_dump_rules()

 include/uapi/linux/netfilter/nf_conntrack_common.h | 2 +-
 net/netfilter/nf_tables_api.c                      | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

^ permalink raw reply

* Re: [PATCH net] of_mdio: avoid MDIO bus removal when a PHY is missing
From: Andrew Lunn @ 2018-01-05 15:12 UTC (permalink / raw)
  To: Madalin Bucur
  Cc: f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1515144974-31377-1-git-send-email-madalin.bucur-3arQi8VN3Tc@public.gmane.org>

On Fri, Jan 05, 2018 at 11:36:14AM +0200, Madalin Bucur wrote:
> If one of the child devices is missing the of_mdiobus_register_phy()
> call will return -ENODEV. When a missing device is encountered the
> registration of the remaining PHYs is stopped and the MDIO bus will
> fail to register. Propagate all errors except ENODEV to avoid it.

Hi Madalin

This is is not clear cut. If the PHY is in device tree, the PHY should
exist. So returning ENODEV is justified. The device tree blob is
broken. But i can also see the value for continuing. There is a chance
some of your other interfaces come up, allowing you to get the correct
device tree blob for the hardware.

Please add

dev_err(&mdio->dev, "MDIO device at address %d is missing.\n");

	Andrew

> 
> Signed-off-by: Madalin Bucur <madalin.bucur-3arQi8VN3Tc@public.gmane.org>
> ---
>  drivers/of/of_mdio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 3481e69..93d41275 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -231,7 +231,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>  			rc = of_mdiobus_register_phy(mdio, child, addr);
>  		else
>  			rc = of_mdiobus_register_device(mdio, child, addr);
> -		if (rc)
> +		if (rc && rc != -ENODEV)
>  			goto unregister;
>  	}
>  
> @@ -255,7 +255,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>  
>  			if (of_mdiobus_child_is_phy(child)) {
>  				rc = of_mdiobus_register_phy(mdio, child, addr);
> -				if (rc)
> +				if (rc && rc != -ENODEV)
>  					goto unregister;
>  			}
>  		}
> -- 
> 2.1.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 8/8] net: tipc: remove unused hardirq.h
From: David Miller @ 2018-01-05 15:17 UTC (permalink / raw)
  To: yang.s
  Cc: linux-kernel, ying.xue, linux-mm, linux-fsdevel, linux-crypto,
	netdev, jon.maloy
In-Reply-To: <b48afbb6-771f-84b1-8329-d5941eff086b@alibaba-inc.com>

From: "Yang Shi" <yang.s@alibaba-inc.com>
Date: Fri, 05 Jan 2018 06:46:48 +0800

> Any more comment on this change?

These patches were not really submitted properly.

If you post a series, the series goes to one destination and
one tree.

If they are supposed to go to multiple trees, submit them
individually rather than as a series.  With clear indications
in the Subject lines which tree should be taking the patch.

Thank you.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH net v3 0/2] SCTP PMTU discovery fixes
From: Neil Horman @ 2018-01-05 15:20 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: netdev, linux-sctp, Xin Long, Vlad Yasevich
In-Reply-To: <cover.1515152627.git.marcelo.leitner@gmail.com>

On Fri, Jan 05, 2018 at 11:17:16AM -0200, Marcelo Ricardo Leitner wrote:
> This patchset fixes 2 issues with PMTU discovery that can lead to flood
> of retransmissions.
> The first patch fixes the issue for when PMTUD is disabled by the
> application, while the second fixes it for when its enabled.
> 
> Please consider these to stable.
> 
> Thanks,
> 
> Marcelo Ricardo Leitner (2):
>   sctp: do not retransmit upon FragNeeded if PMTU discovery is disabled
>   sctp: fix the handling of ICMP Frag Needed for too small MTUs
> 
>  include/net/sctp/structs.h |  2 +-
>  net/sctp/input.c           | 28 ++++++++++++++++------------
>  net/sctp/transport.c       | 29 +++++++++++++++++++----------
>  3 files changed, 36 insertions(+), 23 deletions(-)
> 
> -- 
> 2.14.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Series
Acked-by: Neil Horman <nhorman@tuxdriver.com>

^ permalink raw reply

* Re: [RFC PATCH 1/3] Revert "tcp: allow drivers to tweak TSQ logic"
From: David Miller @ 2018-01-05 15:26 UTC (permalink / raw)
  To: edumazet; +Cc: natale.patriciello, netdev, carloaugusto.grazia
In-Reply-To: <CANn89iL7qzGG8G24dfMqZ1p5hXvLn2N0=Hm-t8n+T7qUCXe9gA@mail.gmail.com>

From: Eric Dumazet <edumazet@google.com>
Date: Fri, 5 Jan 2018 03:44:20 -0800

> On Fri, Jan 5, 2018 at 3:32 AM, Natale Patriciello
> <natale.patriciello@gmail.com> wrote:
>> There are a lot of drivers that are still outside the mainline, and it is
>> impossible to expect the authors (which in most cases do not maintain the
>> driver anymore) to check the best value for the device, and then to insert
>> it into the driver code. Moreover, if the users want to check or to use
>> other values, they have to recompile the driver (or the kernel).
>>
> 
> This is nonsense
> 
> I do not care of out of tree drivers.
> 
> NACK.

Agreed, big NACK from me as well.

^ permalink raw reply

* Re: [PATCH 0/3] Netfilter fixes for net
From: David Miller @ 2018-01-05 15:33 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20180105150825.18460-1-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri,  5 Jan 2018 16:08:22 +0100

> The following patchset contains Netfilter fixes for your net tree,
> they are:
> 
> 1) Fix chain filtering when dumping rules via nf_tables_dump_rules().
> 
> 2) Fix accidental change in NF_CT_STATE_UNTRACKED_BIT through uapi,
>    introduced when removing the untracked conntrack object, from
>    Florian Westphal.
> 
> 3) Fix potential nul-dereference when releasing dump filter in
>    nf_tables_dump_obj_done(), patch from Hangbin Liu.
> 
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thanks Pablo.

^ permalink raw reply

* Re: [PATCH 0/2] Ether fixes for the SolutionEngine771x boards
From: David Miller @ 2018-01-05 15:54 UTC (permalink / raw)
  To: sergei.shtylyov; +Cc: ysato, dalias, linux-sh, netdev
In-Reply-To: <20180103200813.162806401@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 03 Jan 2018 23:08:13 +0300

> Here's the series of 2 patches against Linus' repo. This series should
> (hoplefully) fix the Ether support on the SolutionEngine771x boards...
> 
> [1/2] SolutionEngine771x: fix Ether platform data
> [2/2] SolutionEngine771x: add Ether TSU resource

Looks like this doesn't go through my tree.

Let me know if it should.

^ permalink raw reply

* Re: [PATCH] net/mlx5e: hide an unused variable
From: David Miller @ 2018-01-05 15:56 UTC (permalink / raw)
  To: arnd
  Cc: saeedm, matanb, leonro, ogerlitz, hadarh, paulb, netdev,
	linux-rdma, linux-kernel
In-Reply-To: <20180103224022.3737385-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed,  3 Jan 2018 23:40:11 +0100

> The uplink_rpriv variable was added at the start of the function but
> only used inside of an #ifdef:
> 
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 'mlx5e_route_lookup_ipv6':
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:1549:25: error: unused variable 'uplink_rpriv' [-Werror=unused-variable]
> 
> This moves the declaration into that #ifdef as well.
> 
> Fixes: 5ed99fb421d4 ("net/mlx5e: Move ethernet representors data into separate struct")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

Please make it clear which tree you are targetting in your Subject lines
in the future.  This should have been "[PATCH net-next] ...".

^ permalink raw reply

* Re: [PATCH net-next 0/4] l2tp: remove configurable offset parameters
From: David Miller @ 2018-01-05 16:04 UTC (permalink / raw)
  To: g.nault; +Cc: jchapman, netdev, lorenzo.bianconi, liuhangbin
In-Reply-To: <20180104103228.GH1402@alphalink.fr>

From: Guillaume Nault <g.nault@alphalink.fr>
Date: Thu, 4 Jan 2018 11:32:28 +0100

> On Wed, Jan 03, 2018 at 10:48:03PM +0000, James Chapman wrote:
>> This patch series removes all code to support a configurable offset in
>> transmitted l2tp packets. Code to handle this is incomplete and buggy
>> and has been this way for years. If anyone tried to configure an
>> offset, it would be ignored for L2TPv2 tunnels, or for L2TPv3 tunnels,
>> could result in L2TPv3 packets being transmitted which are not
>> compliant with L2TPv3 RFC3931. This patch series removes the support
>> for configurable offsets.
>> 
> Thanks for the detailed cover letter!
> 
> There are a few more places talking about offsets:
>   * Description of SESSION_CREATE in include/uapi/linux/l2tp.h.
>   * Description of L2TPv3 packet format above l2tp_recv_common() in
>     net/l2tp/l2tp_core.c.
> 
> I can submit a followup patch to remove them.
> 
> Reviewed-by: Guillaume Nault <g.nault@alphalink.fr>
> Tested-by: Guillaume Nault <g.nault@alphalink.fr>

Series applied, thanks everyone.

^ permalink raw reply

* [PATCH net-next] net: phy: fix wrong masks to phy_modify()
From: Russell King @ 2018-01-05 16:07 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev

The mask argument for phy_modify() in several locations was inverted.

Fixes: fea23fb591cc ("net: phy: convert read-modify-write to phy_modify()")
Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/at803x.c     |  2 +-
 drivers/net/phy/marvell.c    | 19 +++++++++----------
 drivers/net/phy/phy-core.c   |  2 +-
 drivers/net/phy/phy_device.c |  6 +++---
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index e86c1b8b1b51..a80cce11b252 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -231,7 +231,7 @@ static int at803x_suspend(struct phy_device *phydev)
 
 static int at803x_resume(struct phy_device *phydev)
 {
-	return phy_modify(phydev, MII_BMCR, ~(BMCR_PDOWN | BMCR_ISOLATE), 0);
+	return phy_modify(phydev, MII_BMCR, BMCR_PDOWN | BMCR_ISOLATE, 0);
 }
 
 static int at803x_probe(struct phy_device *phydev)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 8212c2fd7fe1..5d28063e9327 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -668,7 +668,7 @@ static int m88e3016_config_init(struct phy_device *phydev)
 
 	/* Enable Scrambler and Auto-Crossover */
 	ret = phy_modify(phydev, MII_88E3016_PHY_SPEC_CTRL,
-			 ~MII_88E3016_DISABLE_SCRAMBLER,
+			 MII_88E3016_DISABLE_SCRAMBLER,
 			 MII_88E3016_AUTO_MDIX_CROSSOVER);
 	if (ret < 0)
 		return ret;
@@ -684,9 +684,9 @@ static int m88e1111_config_init_hwcfg_mode(struct phy_device *phydev,
 		mode |= MII_M1111_HWCFG_FIBER_COPPER_AUTO;
 
 	return phy_modify(phydev, MII_M1111_PHY_EXT_SR,
-			  (u16)~(MII_M1111_HWCFG_MODE_MASK |
-				 MII_M1111_HWCFG_FIBER_COPPER_AUTO |
-				 MII_M1111_HWCFG_FIBER_COPPER_RES),
+			  MII_M1111_HWCFG_MODE_MASK |
+			  MII_M1111_HWCFG_FIBER_COPPER_AUTO |
+			  MII_M1111_HWCFG_FIBER_COPPER_RES,
 			  mode);
 }
 
@@ -705,8 +705,7 @@ static int m88e1111_config_init_rgmii_delays(struct phy_device *phydev)
 	}
 
 	return phy_modify(phydev, MII_M1111_PHY_EXT_CR,
-			  (u16)~(MII_M1111_RGMII_RX_DELAY |
-				 MII_M1111_RGMII_TX_DELAY),
+			  MII_M1111_RGMII_RX_DELAY | MII_M1111_RGMII_TX_DELAY,
 			  delay);
 }
 
@@ -833,7 +832,7 @@ static int m88e1510_config_init(struct phy_device *phydev)
 
 		/* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
 		err = phy_modify(phydev, MII_88E1510_GEN_CTRL_REG_1,
-				 ~MII_88E1510_GEN_CTRL_REG_1_MODE_MASK,
+				 MII_88E1510_GEN_CTRL_REG_1_MODE_MASK,
 				 MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII);
 		if (err < 0)
 			return err;
@@ -957,7 +956,7 @@ static int m88e1145_config_init_rgmii(struct phy_device *phydev)
 		if (err < 0)
 			return err;
 
-		err = phy_modify(phydev, 0x1e, 0xf03f,
+		err = phy_modify(phydev, 0x1e, 0x0fc0,
 				 2 << 9 | /* 36 ohm */
 				 2 << 6); /* 39 ohm */
 		if (err < 0)
@@ -1379,7 +1378,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
 
 		/* Setup LED[2] as interrupt pin (active low) */
 		err = __phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
-				   (u16)~MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+				   MII_88E1318S_PHY_LED_TCR_FORCE_INT,
 				   MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
 				   MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
 		if (err < 0)
@@ -1419,7 +1418,7 @@ static int m88e1318_set_wol(struct phy_device *phydev,
 
 		/* Clear WOL status and disable magic packet matching */
 		err = __phy_modify(phydev, MII_88E1318S_PHY_WOL_CTRL,
-				   (u16)~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE,
+				   MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE,
 				   MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS);
 		if (err < 0)
 			goto error;
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 44d09b192014..e75989ce8850 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -332,7 +332,7 @@ EXPORT_SYMBOL(phy_write_mmd);
  * @set: bit mask of bits to set
  *
  * Unlocked helper function which allows a PHY register to be modified as
- * new register value = (old register value & mask) | set
+ * new register value = (old register value & ~mask) | set
  */
 int __phy_modify(struct phy_device *phydev, u32 regnum, u16 mask, u16 set)
 {
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e5ddc5ae56d1..e1acb3052515 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1353,7 +1353,7 @@ EXPORT_SYMBOL(genphy_setup_forced);
 int genphy_restart_aneg(struct phy_device *phydev)
 {
 	/* Don't isolate the PHY if we're negotiating */
-	return phy_modify(phydev, MII_BMCR, ~BMCR_ISOLATE,
+	return phy_modify(phydev, MII_BMCR, BMCR_ISOLATE,
 			  BMCR_ANENABLE | BMCR_ANRESTART);
 }
 EXPORT_SYMBOL(genphy_restart_aneg);
@@ -1626,13 +1626,13 @@ EXPORT_SYMBOL(genphy_suspend);
 
 int genphy_resume(struct phy_device *phydev)
 {
-	return phy_modify(phydev, MII_BMCR, ~BMCR_PDOWN, 0);
+	return phy_modify(phydev, MII_BMCR, BMCR_PDOWN, 0);
 }
 EXPORT_SYMBOL(genphy_resume);
 
 int genphy_loopback(struct phy_device *phydev, bool enable)
 {
-	return phy_modify(phydev, MII_BMCR, ~BMCR_LOOPBACK,
+	return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
 			  enable ? BMCR_LOOPBACK : 0);
 }
 EXPORT_SYMBOL(genphy_loopback);
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 2/2] Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find."
From: Nicolas Dichtel @ 2018-01-05 16:12 UTC (permalink / raw)
  To: Steffen Klassert, David Miller; +Cc: herbert, robsonde, netdev
In-Reply-To: <20171223160938.rmfwopbmeyepndh5@gauss3.secunet.de>

Le 23/12/2017 à 17:09, Steffen Klassert a écrit :
> On Sat, Dec 23, 2017 at 10:56:12AM -0500, David Miller wrote:
>> From: Steffen Klassert <steffen.klassert@secunet.com>
>> Date: Sat, 23 Dec 2017 10:22:17 +0100
>>
>>> On Thu, Nov 16, 2017 at 11:00:40AM +0100, Steffen Klassert wrote:
>>>> This reverts commit c9f3f813d462c72dbe412cee6a5cbacf13c4ad5e.
>>>>
>>>> This commit breaks transport mode when the policy template
>>>> has widlcard addresses configured, so revert it.
>>>>
>>>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>>>
>>> David, can you please queue this one up for v4.14-stable?
>>> Commit ID is 94802151894d482e82c324edf2c658f8e6b96508
>>>
>>> v4.14 is unusable for some people without this revert.
>>
>> Yes, but it adds back the stack out-of-bounds bug.
>>
>> If I queue up the revert, I would also need to queue up whatever
>> follow-on you used to fix the out-of-bounds bug properly.  Which
>> commit is that?
> 
> This is commit ddc47e4404b58f03e98345398fb12d38fe291512
> ("xfrm: Fix stack-out-of-bounds read on socket policy lookup.")
> 
> It is included in the pull request for the net tree that
> I sent yesterday. The patch looks save, but not so sure
> if it should go directly to stable. These bugs reported by
> the syzbot are usually quite subtile and I already broke
> something when I tried to fix the original stack out-of-bounds
> bug. So maybe we should wait until the v4.15 release before
> backporting...
> 
This patch is still missing in the 4.14 stable. Without it, some IPsec scenarii
are broken. Is there a plan to queue this patch for the 4.14 stable ?


Thank you,
Nicolas

^ permalink raw reply

* Re: [PATCH net-next 1/2] ip: do not set RFS core on error queue reads
From: David Miller @ 2018-01-05 16:15 UTC (permalink / raw)
  To: soheil.kdev; +Cc: netdev, pjt, ycheng, soheil, willemb, edumazet, ncardwell
In-Reply-To: <20180104024711.257600-1-soheil.kdev@gmail.com>

From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
Date: Wed,  3 Jan 2018 21:47:10 -0500

> From: Soheil Hassas Yeganeh <soheil@google.com>
> 
> We should only record RPS on normal reads and writes.
> In single threaded processes, all calls record the same state. In
> multi-threaded processes where a separate thread processes
> errors, the RFS table mispredicts.
> 
> Note that, when CONFIG_RPS is disabled, sock_rps_record_flow
> is a noop and no branch is added as a result of this patch.
> 
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: revert "Update RFS target at poll for tcp/udp"
From: David Miller @ 2018-01-05 16:15 UTC (permalink / raw)
  To: soheil.kdev; +Cc: netdev, pjt, ycheng, soheil, willemb, edumazet, ncardwell
In-Reply-To: <20180104024711.257600-2-soheil.kdev@gmail.com>

From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
Date: Wed,  3 Jan 2018 21:47:11 -0500

> From: Soheil Hassas Yeganeh <soheil@google.com>
> 
> On multi-threaded processes, one common architecture is to have
> one (or a small number of) threads polling sockets, and a
> considerably larger pool of threads reading form and writing to the
> sockets. When we set RPS core on tcp_poll() or udp_poll() we essentially
> steer all packets of all the polled FDs to one (or small number of)
> cores, creaing a bottleneck and/or RPS misprediction.
> 
> Another common architecture is to shard FDs among threads pinned
> to cores. In such a setting, setting RPS core in tcp_poll() and
> udp_poll() is redundant because the RFS core is correctly
> set in recvmsg and sendmsg.
> 
> Thus, revert the following commit:
> c3f1dbaf6e28 ("net: Update RFS target at poll for tcp/udp").
> 
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] net: sched: fix tcf_block_get_ext() in case CONFIG_NET_CLS is not set
From: David Miller @ 2018-01-05 16:18 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers, jiri, aring, quentin.monnet
In-Reply-To: <20180104013045.29510-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed,  3 Jan 2018 17:30:45 -0800

> From: Quentin Monnet <quentin.monnet@netronome.com>
> 
> The definition of functions tcf_block_get() and tcf_block_get_ext()
> depends of CONFIG_NET_CLS being set. When those functions gained extack
> support, only one version of the declaration of those functions was
> updated. Function tcf_block_get() was later fixed with commit
> 3c1490913f3b ("net: sch: api: fix tcf_block_get").
> 
> Change arguments of tcf_block_get_ext() for the case when CONFIG_NET_CLS
> is not set.
> 
> Fixes: 8d1a77f974ca ("net: sch: api: add extack support in tcf_block_get")
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net,stable 1/1] net: fec: free/restore resource in related probe error pathes
From: David Miller @ 2018-01-05 16:20 UTC (permalink / raw)
  To: fugang.duan; +Cc: troy.kisky, netdev, festevam
In-Reply-To: <1515034040-12482-1-git-send-email-fugang.duan@nxp.com>

From: Fugang Duan <fugang.duan@nxp.com>
Date: Thu, 4 Jan 2018 10:47:20 +0800

> Fixes in probe error path:
> - Restore dev_id before failed_ioremap path.
>   Fixes: ("net: fec: restore dev_id in the cases of probe error")
> - Call of_node_put(phy_node) before failed_phy path.
>   Fixes: ("net: fec: Support phys probed from devicetree and fixed-link")
> 
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next V2 2/2] tun: allow to attach ebpf socket filter
From: Willem de Bruijn @ 2018-01-05 16:21 UTC (permalink / raw)
  To: Jason Wang
  Cc: Network Development, LKML, Michael S. Tsirkin, Willem de Bruijn
In-Reply-To: <1515124488-396-3-git-send-email-jasowang@redhat.com>

On Fri, Jan 5, 2018 at 4:54 AM, Jason Wang <jasowang@redhat.com> wrote:
> This patch allows userspace to attach eBPF filter to tun. This will
> allow to implement VM dataplane filtering in a more efficient way
> compared to cBPF filter by allowing either qemu or libvirt to
> attach eBPF filter to tun.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/net/tun.c           | 39 +++++++++++++++++++++++++++++++++++----
>  include/uapi/linux/if_tun.h |  1 +
>  2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 0853829..9fc8b70 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -238,6 +238,12 @@ struct tun_struct {
>         struct tun_pcpu_stats __percpu *pcpu_stats;
>         struct bpf_prog __rcu *xdp_prog;
>         struct tun_prog __rcu *steering_prog;
> +       struct tun_prog __rcu *filter_prog;
> +};
> +
> +struct veth {
> +       __be16 h_vlan_proto;
> +       __be16 h_vlan_TCI;
>  };
>
>  static int tun_napi_receive(struct napi_struct *napi, int budget)
> @@ -984,12 +990,25 @@ static void tun_automq_xmit(struct tun_struct *tun, struct sk_buff *skb)
>  #endif
>  }
>
> +static unsigned int run_ebpf_filter(struct tun_struct *tun,
> +                                   struct sk_buff *skb,
> +                                   int len)
> +{
> +       struct tun_prog *prog = rcu_dereference(tun->filter_prog);
> +
> +       if (prog)
> +               len = bpf_prog_run_clear_cb(prog->prog, skb);
> +
> +       return len;
> +}
> +
>  /* Net device start xmit */
>  static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
>         struct tun_struct *tun = netdev_priv(dev);
>         int txq = skb->queue_mapping;
>         struct tun_file *tfile;
> +       int len = skb->len;
>
>         rcu_read_lock();
>         tfile = rcu_dereference(tun->tfiles[txq]);
> @@ -1015,6 +1034,16 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>             sk_filter(tfile->socket.sk, skb))
>                 goto drop;
>
> +       len = run_ebpf_filter(tun, skb, len);
> +
> +       /* Trim extra bytes since we may inster vlan proto & TCI

inster -> insert

> +        * in tun_put_user().
> +        */
> +       if (skb_vlan_tag_present(skb))
> +               len -= skb_vlan_tag_present(skb) ? sizeof(struct veth) : 0;

no need for testing skb_vlan_tag_present twice.

more importantly, why trim these bytes unconditionally?

only if the filter trims a packet to a length shorter than the the minimum
could this cause problems. sk_filter_trim_cap with a lower bound avoids
that: skb_vlan_tag_present(skb) ? sizeof(struct vlan_ethhdr) : 0;

^ permalink raw reply

* Re: [PATCH net-next 0/3] net: dsa: Move padding into Broadcom tagger
From: David Miller @ 2018-01-05 16:21 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, andrew, vivien.didelot
In-Reply-To: <20180104061302.27631-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed,  3 Jan 2018 22:12:59 -0800

> This patch series moves the padding of short packets to where it belongs
> within the DSA Broadcom tagger code, I just found myself doing this for
> a third driver, which was a clear indication this was wrong and did not
> scale.

Series applied, thanks Florian.

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: lan9303: Fix error return code in lan9303_check_device()
From: David Miller @ 2018-01-05 16:25 UTC (permalink / raw)
  To: weiyongjun1; +Cc: andrew, vivien.didelot, f.fainelli, netdev
In-Reply-To: <1515051028-59363-1-git-send-email-weiyongjun1@huawei.com>

From: Wei Yongjun <weiyongjun1@huawei.com>
Date: Thu, 4 Jan 2018 07:30:28 +0000

> Fix to return error code -ENODEV from the chip not found error handling
> case instead of 0(ret have been overwritten to 0 by lan9303_read()), as
> done elsewhere in this function.
> 
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/6] move _body_io_syscall to the generic syscall.h
From: Jeff Moyer @ 2018-01-05 16:25 UTC (permalink / raw)
  To: Christoph Hellwig, Benjamin LaHaise
  Cc: linux-aio, Avi Kivity, linux-fsdevel, netdev, linux-kernel
In-Reply-To: <20180104080325.14716-3-hch@lst.de>

Christoph Hellwig <hch@lst.de> writes:

> This way it can be used for the fallback 6-argument version on
> all architectures.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This is a strange way to do things.  However, I was never really sold on
libaio having to implement its own system call wrappers.  That decision
definitely resulted in some maintenance overhead.

Ben, what was your reasoning for not just using syscall?

-Jeff

> ---
>  src/syscall-generic.h | 6 ------
>  src/syscall.h         | 7 +++++++
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/syscall-generic.h b/src/syscall-generic.h
> index 24d7c7c..35b8580 100644
> --- a/src/syscall-generic.h
> +++ b/src/syscall-generic.h
> @@ -2,12 +2,6 @@
>  #include <unistd.h>
>  #include <sys/syscall.h>
>  
> -#define _body_io_syscall(sname, args...) \
> -{ \
> -	int ret = syscall(__NR_##sname, ## args); \
> -	return ret < 0 ? -errno : ret; \
> -}
> -
>  #define io_syscall1(type,fname,sname,type1,arg1) \
>  type fname(type1 arg1) \
>  _body_io_syscall(sname, (long)arg1)
> diff --git a/src/syscall.h b/src/syscall.h
> index a2da030..3819519 100644
> --- a/src/syscall.h
> +++ b/src/syscall.h
> @@ -10,6 +10,13 @@
>  #define DEFSYMVER(compat_sym, orig_sym, ver_sym)	\
>  	__asm__(".symver " SYMSTR(compat_sym) "," SYMSTR(orig_sym) "@@LIBAIO_" SYMSTR(ver_sym));
>  
> +/* generic fallback */
> +#define _body_io_syscall(sname, args...) \
> +{ \
> +	int ret = syscall(__NR_##sname, ## args); \
> +	return ret < 0 ? -errno : ret; \
> +}
> +
>  #if defined(__i386__)
>  #include "syscall-i386.h"
>  #elif defined(__x86_64__)

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [PATCH 2/6] move _body_io_syscall to the generic syscall.h
From: Benjamin LaHaise @ 2018-01-05 16:40 UTC (permalink / raw)
  To: Jeff Moyer
  Cc: Christoph Hellwig, linux-aio, Avi Kivity, linux-fsdevel, netdev,
	linux-kernel
In-Reply-To: <x49tvw0z30y.fsf@segfault.boston.devel.redhat.com>

On Fri, Jan 05, 2018 at 11:25:17AM -0500, Jeff Moyer wrote:
> Christoph Hellwig <hch@lst.de> writes:
> 
> > This way it can be used for the fallback 6-argument version on
> > all architectures.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> This is a strange way to do things.  However, I was never really sold on
> libaio having to implement its own system call wrappers.  That decision
> definitely resulted in some maintenance overhead.
> 
> Ben, what was your reasoning for not just using syscall?

The main issue was that glibc's pthreads implementation really sucked back
during initial development and there was a use-case for having the io_XXX
functions usable directly from clone()ed threads that didn't have all the
glibc pthread state setup for per-cpu areas to handle per-thread errno.
That made sense back then, but is rather silly today.

Technically, I'm not sure the generic syscall wrapper is safe to use.  The
io_XXX arch wrappers don't modify errno, while it appears the generic one
does.  That said, nobody has ever noticed...

		-ben

> -Jeff
> 
> > ---
> >  src/syscall-generic.h | 6 ------
> >  src/syscall.h         | 7 +++++++
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/syscall-generic.h b/src/syscall-generic.h
> > index 24d7c7c..35b8580 100644
> > --- a/src/syscall-generic.h
> > +++ b/src/syscall-generic.h
> > @@ -2,12 +2,6 @@
> >  #include <unistd.h>
> >  #include <sys/syscall.h>
> >  
> > -#define _body_io_syscall(sname, args...) \
> > -{ \
> > -	int ret = syscall(__NR_##sname, ## args); \
> > -	return ret < 0 ? -errno : ret; \
> > -}
> > -
> >  #define io_syscall1(type,fname,sname,type1,arg1) \
> >  type fname(type1 arg1) \
> >  _body_io_syscall(sname, (long)arg1)
> > diff --git a/src/syscall.h b/src/syscall.h
> > index a2da030..3819519 100644
> > --- a/src/syscall.h
> > +++ b/src/syscall.h
> > @@ -10,6 +10,13 @@
> >  #define DEFSYMVER(compat_sym, orig_sym, ver_sym)	\
> >  	__asm__(".symver " SYMSTR(compat_sym) "," SYMSTR(orig_sym) "@@LIBAIO_" SYMSTR(ver_sym));
> >  
> > +/* generic fallback */
> > +#define _body_io_syscall(sname, args...) \
> > +{ \
> > +	int ret = syscall(__NR_##sname, ## args); \
> > +	return ret < 0 ? -errno : ret; \
> > +}
> > +
> >  #if defined(__i386__)
> >  #include "syscall-i386.h"
> >  #elif defined(__x86_64__)
> 

-- 
"Thought is the essence of where you are now."

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [PATCHv5 1/3] dt-bindings: net: Add DT bindings for Socionext Netsec
From: Jassi Brar @ 2018-01-05 16:41 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: <netdev@vger.kernel.org>, Devicetree List, David S. Miller,
	Arnd Bergmann, Andrew Lunn, Rob Herring, Mark Rutland,
	Masami Hiramatsu, Jassi Brar
In-Reply-To: <CAKv+Gu9Pk_WEOrWZdVK5ty9GaDm4ApvGeksu_6W5DcF0-SOUuA@mail.gmail.com>

Hi Ard,

On Thu, Jan 4, 2018 at 3:07 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> Hi Jassi,
>
> On 1 January 2018 at 05:14,  <jassisinghbrar@gmail.com> wrote:
>> From: Jassi Brar <jassisinghbrar@gmail.com>
>>
>> This patch adds documentation for Device-Tree bindings for the
>> Socionext NetSec Controller driver.
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  .../devicetree/bindings/net/socionext-netsec.txt   | 53 ++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/socionext-netsec.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/socionext-netsec.txt b/Documentation/devicetree/bindings/net/socionext-netsec.txt
>> new file mode 100644
>> index 0000000..b70e35b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/socionext-netsec.txt
>> @@ -0,0 +1,53 @@
>> +* Socionext NetSec Ethernet Controller IP
>> +
>> +Required properties:
>> +- compatible: Should be "socionext,synquacer-netsec"
>> +- reg: Address and length of the control register area, followed by the
>> +       address and length of the EEPROM holding the MAC address and
>> +       microengine firmware
>> +- interrupts: Should contain ethernet controller interrupt
>> +- clocks: phandle to the PHY reference clock
>> +- clock-names: Should be "phy_ref_clk"
>
> If clock-names is mandatory now even when only a single clock is
> specified,
>
I know. Usually I follow maintainer's opinion unless I see myself
losing sleep doing that.

> you should add it to the example as well.
>
Done. Thanks

> However, please
> be aware that hardware has shipped now with DT images that specify a
> only a single clock and no clock-names property: those will require a
> firmware upgrade before they can use this version of the driver.
>
Ok, for now I have dared ignore clock-names in driver.... until we
have to support again instance that take in more than one clock.

Thanks.

^ permalink raw reply

* Re: [PATCHv5 2/3] net: socionext: Add Synquacer NetSec driver
From: Jassi Brar @ 2018-01-05 16:53 UTC (permalink / raw)
  To: David Miller
  Cc: Jassi Brar, netdev, Devicetree List, Arnd Bergmann, Andrew Lunn,
	Ard Biesheuvel, Rob Herring, Mark Rutland, Masami Hiramatsu
In-Reply-To: <20180103.103546.528269429969753470.davem@davemloft.net>

On 3 January 2018 at 21:05, David Miller <davem@davemloft.net> wrote:
> From: jassisinghbrar@gmail.com
> Date: Mon,  1 Jan 2018 10:44:49 +0530
>
>> +#define DRING_TAIL(r)                ((r)->tail)
>> +
>> +#define DRING_HEAD(r)                ((r)->head)
>
> These macros do not help readability at all.
>
>> +#define MOVE_TAIL(r)         do { \
>> +                                     if (++(r)->tail == DESC_NUM) \
>> +                                             (r)->tail = 0; \
>> +                             } while (0)
>> +
>> +#define MOVE_HEAD(r)         do { \
>> +                                     if (++(r)->head == DESC_NUM) \
>> +                                             (r)->head = 0; \
>> +                             } while (0)
>> +
>> +#define JUMP_HEAD(r, n)      do { \
>> +                                     int i; \
>> +                                     for (i = 0; i < (n); i++) \
>> +                                             MOVE_HEAD(r); \
>> +                             } while (0)
>
> Neither do these.
>
OK, I have removed these.

> And JUMP_HEAD is so inefficient, it's a constant time calculation:
>
>         r->head += n;
>         if (r->head >= DESC_NUM)
>                 r->head -= DESC_NUM;
>
How about the more compact    r->head = (r->head + n) % DESC_NUM

> All of this stuff can be done inline without all of these CPP macros
> which are discouraged, have multiple evaluation issues, and decrease
> the amount of type checking going on.
>
> If you absolutely must have helpers, use static functions (without
> the inline keyword, let the compiler device).
>
>> +static inline int available_descs(struct netsec_desc_ring *r)
>
> No inline functions in foo.c files, let the compiler device.
>
OK. I have already started selling it downstream.

>> +/*************************************************************/
>> +/*********************** NETDEV_OPS **************************/
>> +/*************************************************************/
>
> Please, comments are not billboards or Star Wars openning sequences.
> Simplify this, thank you.
>
Done  :)

>> +
>> +static void netsec_set_tx_de(struct netsec_priv *priv,
>> +                          struct netsec_desc_ring *dring,
>> +                          const struct netsec_tx_pkt_ctrl *tx_ctrl,
>> +                          const struct netsec_desc *desc,
>> +                          struct sk_buff *skb)
>> +{
>> +     struct netsec_de *de;
>> +     int idx = DRING_HEAD(dring);
>> +     u32 attr;
>
> Please order local variables from longest to shortest line.
>
> Please audit your entire submission for this issue.
>
That was the only occurrence. I already am particular about following
that style.

Thank you.

^ 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