Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1291569 > unrolled thread

[PATCH net-next v5 00/19] new ETHTOOL_GSETTINGS/SSETTINGS API

Started byDavid Decotigny <ddecotig@gmail.com>
First post2015-12-14 22:10 +0100
Last post2015-12-17 01:30 +0100
Articles 9 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH net-next v5 00/19] new ETHTOOL_GSETTINGS/SSETTINGS API David Decotigny <ddecotig@gmail.com> - 2015-12-14 22:10 +0100
    [PATCH net-next v5 19/19] net: mlx4: use new ETHTOOL_G/SSETTINGS API David Decotigny <ddecotig@gmail.com> - 2015-12-14 22:10 +0100
    [PATCH net-next v5 17/19] net: ethtool: remove unused __ethtool_get_settings David Decotigny <ddecotig@gmail.com> - 2015-12-14 22:10 +0100
    [PATCH net-next v5 02/19] test_bitmap: unit tests for lib/bitmap.c David Decotigny <ddecotig@gmail.com> - 2015-12-14 22:20 +0100
    [PATCH net-next v5 03/19] net: usnic: remove unused call to ethtool_ops::get_settings David Decotigny <ddecotig@gmail.com> - 2015-12-14 22:20 +0100
    [PATCH net-next v5 07/19] net: usnic: use __ethtool_get_ksettings David Decotigny <ddecotig@gmail.com> - 2015-12-14 22:20 +0100
    [PATCH net-next v5 06/19] tx4939: use __ethtool_get_ksettings David Decotigny <ddecotig@gmail.com> - 2015-12-14 22:20 +0100
    Re: [PATCH net-next v5 05/19] net: ethtool: add new  ETHTOOL_GSETTINGS/SSETTINGS API David Miller <davem@davemloft.net> - 2015-12-16 17:40 +0100
      Re: [PATCH net-next v5 05/19] net: ethtool: add new  ETHTOOL_GSETTINGS/SSETTINGS API David Decotigny <ddecotig@gmail.com> - 2015-12-17 01:30 +0100

#1291569 — [PATCH net-next v5 00/19] new ETHTOOL_GSETTINGS/SSETTINGS API

FromDavid Decotigny <ddecotig@gmail.com>
Date2015-12-14 22:10 +0100
Subject[PATCH net-next v5 00/19] new ETHTOOL_GSETTINGS/SSETTINGS API
Message-ID<qFIxz-34I-7@gated-at.bofh.it>
From: David Decotigny <decot@googlers.com>


History:
 v5
 note: please see v4 bullets for a question regarding bitmap.c
 - minor fix to make allyesconfig/allmodconfig
 v4
 - removed typedef for link mode bitmaps
 - moved bitmap<->u32[] conversion routines to bitmap.c . This is the
   naive implementation. I have an endian-aware version that uses
   memcpy/memset as much as possible, but I find it harder to follow
   (see http://paste.ubuntu.com/13863722/). Please let me know if I
   should use it instead.
 - fixes suggested by Ben Hutchings
 v3
 - rebased v2 on top of latest net-next, minor checkpatch/printf %*pb
   updates
 v2
 - keep return 0 in get_settings when successful, instead of
   propagating positive result from driver's get_settings callback.
 v1
 - original submission


The main goal of this series is to support ethtool link mode masks
larger than 32 bits. It implements a new ioctl pair
(ETHTOOL_GSETTINGS/SSETTINGS), its associated callbacks
(get/set_settings) and a new struct ethtool_settings, which should
eventually replace legacy ethtool_cmd. Internally, the kernel uses
fixed length link mode masks defined at compilation time in ethtool.h
(for now: 31 bits), that can be increased by changing
__ETHTOOL_LINK_MODE_LAST in ethtool.h (absolute max is 4064 bits,
checked at compile time), and the user/kernel interface allows this
length to be arbitrary within 1..4064. This should allow some
flexibility without using too much heap/stack space, at the cost of
a small kernel/user handshake for the user to determine the sizes of
those bitmaps.

Along the way, I chose to drop in the new structure the 3 ethtool_cmd
fields marked "deprecated" (transceiver/maxrxpkt/maxtxpkt). They are
still available for old drivers via the (old) ETHTOOL_GSET/SSET API,
but are not available to drivers that switch to new API. Of those 3
fields, ethtool_cmd::transceiver seems to be still actively used by
several drivers, maybe we should not consider this field deprecated?
The 2 other fields are basically not used. This transition requires
some care in the way old and new ethtool talk to the kernel.

More technical details provided in the description for main patch. In
particular details about backward compatibility properties.

Some questions to more experts than me:
 - the kernel/interface multiplexes the "tell me the bitmap length"
   handshake and the "give me the settings" inside the new
   ETHTOOL_GSETTINGS cmd. I was thinking of making this into 2
   separate cmds: 1 cmd ETHTOOL_GKERNELPROPERTIES which would be
   kernel-wide rather than device-specific, would return properties
   like "length of the link mode bitmaps", and possibly others. And
   ETHTOOL_GSETTINGS would expect the proper bitmaps
 - the link mode bitmaps are piggybacked at tail of the new struct
   ethtool_settings. Since its user-visible definition does not assume
   specific bitmap width, I am using a 0-length array as the publicly
   visible placeholder. But then, the kernel needs to specialize it
   (struct ethtool_ksettings) to specify its current link mode
   masks. This means that kernel code is "littered" with
   "ksettings->parent.field" to access "field" inside
   ethtool_settings:
   + I don't like the field name "parent", any suggestion welcome
   + and/or: I could use ethtool_settings everywhere (instead of a new
     ethtool_ksettings) and an accessor to retrieve the link mode
     masks?
   + or: we could decide to make the link mode masks statically
     bounded again, ie. make their width public, but larger than
     current 32, and unchangeable forever. This would make everything
     straightforward, but we might hit limits later, or have an
     unneeded memory/stack usage for unused bits.
   any preference?
 - I foresee bugs where people use the legacy/deprecated SUPPORTED_x
   macros instead of the new ETHTOOL_LINK_MODE_x_BIT enums in the new
   get/set__ksettings callbacks. Not sure how to prevent problems with
   this.

The only driver which was converted for now is mlx4. I am not
considering fcoe as fully converted, but I updated it a minima to be
able to remove __ethtool_get_settings, now known as
__ethtool_get_ksettings.

Tested with legacy and "future" ethtool on 64b x86 kernel and 32+64b
ethtool, and on a 32b x86 kernel + 32b ethtool.

############################################
# Patch Set Summary:

David Decotigny (19):
  lib/bitmap.c: conversion routines to/from u32 array
  test_bitmap: unit tests for lib/bitmap.c
  net: usnic: remove unused call to ethtool_ops::get_settings
  net: usnic: use __ethtool_get_settings
  net: ethtool: add new ETHTOOL_GSETTINGS/SSETTINGS API
  tx4939: use __ethtool_get_ksettings
  net: usnic: use __ethtool_get_ksettings
  net: bonding: use __ethtool_get_ksettings
  net: ipvlan: use __ethtool_get_ksettings
  net: macvlan: use __ethtool_get_ksettings
  net: team: use __ethtool_get_ksettings
  net: fcoe: use __ethtool_get_ksettings
  net: rdma: use __ethtool_get_ksettings
  net: 8021q: use __ethtool_get_ksettings
  net: bridge: use __ethtool_get_ksettings
  net: core: use __ethtool_get_ksettings
  net: ethtool: remove unused __ethtool_get_settings
  net: mlx4: convenience predicate for debug messages
  net: mlx4: use new ETHTOOL_G/SSETTINGS API

 arch/mips/txx9/generic/setup_tx4939.c           |   7 +-
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c    |  10 +-
 drivers/net/bonding/bond_main.c                 |  14 +-
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 344 ++++++++++----------
 drivers/net/ethernet/mellanox/mlx4/en_main.c    |   1 +
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    |   5 +-
 drivers/net/ipvlan/ipvlan_main.c                |   8 +-
 drivers/net/macvlan.c                           |   8 +-
 drivers/net/team/team.c                         |   8 +-
 drivers/scsi/fcoe/fcoe_transport.c              |  36 ++-
 include/linux/bitmap.h                          |   6 +
 include/linux/ethtool.h                         |  84 ++++-
 include/rdma/ib_addr.h                          |  14 +-
 include/uapi/linux/ethtool.h                    | 320 +++++++++++++++----
 lib/Kconfig.debug                               |   8 +
 lib/Makefile                                    |   1 +
 lib/bitmap.c                                    |  86 +++++
 lib/test_bitmap.c                               | 343 ++++++++++++++++++++
 net/8021q/vlan_dev.c                            |   8 +-
 net/bridge/br_if.c                              |   6 +-
 net/core/ethtool.c                              | 400 +++++++++++++++++++++++-
 net/core/net-sysfs.c                            |  15 +-
 net/packet/af_packet.c                          |  11 +-
 tools/testing/selftests/lib/Makefile            |   2 +-
 tools/testing/selftests/lib/bitmap.sh           |  10 +
 25 files changed, 1427 insertions(+), 328 deletions(-)
 create mode 100644 lib/test_bitmap.c
 create mode 100644 tools/testing/selftests/lib/bitmap.sh

-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1291570 — [PATCH net-next v5 19/19] net: mlx4: use new ETHTOOL_G/SSETTINGS API

FromDavid Decotigny <ddecotig@gmail.com>
Date2015-12-14 22:10 +0100
Subject[PATCH net-next v5 19/19] net: mlx4: use new ETHTOOL_G/SSETTINGS API
Message-ID<qFIxB-34I-47@gated-at.bofh.it>
In reply to#1291569
From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 344 ++++++++++++------------
 drivers/net/ethernet/mellanox/mlx4/en_main.c    |   1 +
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h    |   1 +
 3 files changed, 177 insertions(+), 169 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index dd84cab..f33f27b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -501,34 +501,30 @@ static u32 mlx4_en_autoneg_get(struct net_device *dev)
 	return autoneg;
 }
 
-static u32 ptys_get_supported_port(struct mlx4_ptys_reg *ptys_reg)
+static void ptys2ethtool_update_supported_port(unsigned long *mask,
+					       struct mlx4_ptys_reg *ptys_reg)
 {
 	u32 eth_proto = be32_to_cpu(ptys_reg->eth_proto_cap);
 
 	if (eth_proto & (MLX4_PROT_MASK(MLX4_10GBASE_T)
 			 | MLX4_PROT_MASK(MLX4_1000BASE_T)
 			 | MLX4_PROT_MASK(MLX4_100BASE_TX))) {
-			return SUPPORTED_TP;
-	}
-
-	if (eth_proto & (MLX4_PROT_MASK(MLX4_10GBASE_CR)
+		__set_bit(ETHTOOL_LINK_MODE_TP_BIT, mask);
+	} else if (eth_proto & (MLX4_PROT_MASK(MLX4_10GBASE_CR)
 			 | MLX4_PROT_MASK(MLX4_10GBASE_SR)
 			 | MLX4_PROT_MASK(MLX4_56GBASE_SR4)
 			 | MLX4_PROT_MASK(MLX4_40GBASE_CR4)
 			 | MLX4_PROT_MASK(MLX4_40GBASE_SR4)
 			 | MLX4_PROT_MASK(MLX4_1000BASE_CX_SGMII))) {
-			return SUPPORTED_FIBRE;
-	}
-
-	if (eth_proto & (MLX4_PROT_MASK(MLX4_56GBASE_KR4)
+		__set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, mask);
+	} else if (eth_proto & (MLX4_PROT_MASK(MLX4_56GBASE_KR4)
 			 | MLX4_PROT_MASK(MLX4_40GBASE_KR4)
 			 | MLX4_PROT_MASK(MLX4_20GBASE_KR2)
 			 | MLX4_PROT_MASK(MLX4_10GBASE_KR)
 			 | MLX4_PROT_MASK(MLX4_10GBASE_KX4)
 			 | MLX4_PROT_MASK(MLX4_1000BASE_KX))) {
-			return SUPPORTED_Backplane;
+		__set_bit(ETHTOOL_LINK_MODE_Backplane_BIT, mask);
 	}
-	return 0;
 }
 
 static u32 ptys_get_active_port(struct mlx4_ptys_reg *ptys_reg)
@@ -574,122 +570,111 @@ static u32 ptys_get_active_port(struct mlx4_ptys_reg *ptys_reg)
 enum ethtool_report {
 	SUPPORTED = 0,
 	ADVERTISED = 1,
-	SPEED = 2
 };
 
+struct ptys2ethtool_config {
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(advertised);
+	u32 speed;
+};
+
+static unsigned long *ptys2ethtool_link_mode(struct ptys2ethtool_config *cfg,
+					     enum ethtool_report report)
+{
+	switch (report) {
+	case SUPPORTED:
+		return cfg->supported;
+	case ADVERTISED:
+		return cfg->advertised;
+	}
+	return NULL;
+}
+
+#define MLX4_BUILD_PTYS2ETHTOOL_CONFIG(reg_, speed_, ...)		\
+	({								\
+		struct ptys2ethtool_config *cfg;			\
+		const unsigned int modes[] = { __VA_ARGS__ };		\
+		unsigned int i;						\
+		cfg = &ptys2ethtool_map[reg_];				\
+		cfg->speed = speed_;					\
+		bitmap_zero(cfg->supported,				\
+			    __ETHTOOL_LINK_MODE_MASK_NBITS);		\
+		bitmap_zero(cfg->advertised,				\
+			    __ETHTOOL_LINK_MODE_MASK_NBITS);		\
+		for (i = 0 ; i < ARRAY_SIZE(modes) ; ++i) {		\
+			__set_bit(modes[i], cfg->supported);		\
+			__set_bit(modes[i], cfg->advertised);		\
+		}							\
+	})
+
 /* Translates mlx4 link mode to equivalent ethtool Link modes/speed */
-static u32 ptys2ethtool_map[MLX4_LINK_MODES_SZ][3] = {
-	[MLX4_100BASE_TX] = {
-		SUPPORTED_100baseT_Full,
-		ADVERTISED_100baseT_Full,
-		SPEED_100
-		},
-
-	[MLX4_1000BASE_T] = {
-		SUPPORTED_1000baseT_Full,
-		ADVERTISED_1000baseT_Full,
-		SPEED_1000
-		},
-	[MLX4_1000BASE_CX_SGMII] = {
-		SUPPORTED_1000baseKX_Full,
-		ADVERTISED_1000baseKX_Full,
-		SPEED_1000
-		},
-	[MLX4_1000BASE_KX] = {
-		SUPPORTED_1000baseKX_Full,
-		ADVERTISED_1000baseKX_Full,
-		SPEED_1000
-		},
-
-	[MLX4_10GBASE_T] = {
-		SUPPORTED_10000baseT_Full,
-		ADVERTISED_10000baseT_Full,
-		SPEED_10000
-		},
-	[MLX4_10GBASE_CX4] = {
-		SUPPORTED_10000baseKX4_Full,
-		ADVERTISED_10000baseKX4_Full,
-		SPEED_10000
-		},
-	[MLX4_10GBASE_KX4] = {
-		SUPPORTED_10000baseKX4_Full,
-		ADVERTISED_10000baseKX4_Full,
-		SPEED_10000
-		},
-	[MLX4_10GBASE_KR] = {
-		SUPPORTED_10000baseKR_Full,
-		ADVERTISED_10000baseKR_Full,
-		SPEED_10000
-		},
-	[MLX4_10GBASE_CR] = {
-		SUPPORTED_10000baseKR_Full,
-		ADVERTISED_10000baseKR_Full,
-		SPEED_10000
-		},
-	[MLX4_10GBASE_SR] = {
-		SUPPORTED_10000baseKR_Full,
-		ADVERTISED_10000baseKR_Full,
-		SPEED_10000
-		},
-
-	[MLX4_20GBASE_KR2] = {
-		SUPPORTED_20000baseMLD2_Full | SUPPORTED_20000baseKR2_Full,
-		ADVERTISED_20000baseMLD2_Full | ADVERTISED_20000baseKR2_Full,
-		SPEED_20000
-		},
-
-	[MLX4_40GBASE_CR4] = {
-		SUPPORTED_40000baseCR4_Full,
-		ADVERTISED_40000baseCR4_Full,
-		SPEED_40000
-		},
-	[MLX4_40GBASE_KR4] = {
-		SUPPORTED_40000baseKR4_Full,
-		ADVERTISED_40000baseKR4_Full,
-		SPEED_40000
-		},
-	[MLX4_40GBASE_SR4] = {
-		SUPPORTED_40000baseSR4_Full,
-		ADVERTISED_40000baseSR4_Full,
-		SPEED_40000
-		},
-
-	[MLX4_56GBASE_KR4] = {
-		SUPPORTED_56000baseKR4_Full,
-		ADVERTISED_56000baseKR4_Full,
-		SPEED_56000
-		},
-	[MLX4_56GBASE_CR4] = {
-		SUPPORTED_56000baseCR4_Full,
-		ADVERTISED_56000baseCR4_Full,
-		SPEED_56000
-		},
-	[MLX4_56GBASE_SR4] = {
-		SUPPORTED_56000baseSR4_Full,
-		ADVERTISED_56000baseSR4_Full,
-		SPEED_56000
-		},
+static struct ptys2ethtool_config ptys2ethtool_map[MLX4_LINK_MODES_SZ];
+
+void __init mlx4_en_init_ptys2ethtool_map(void)
+{
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_100BASE_TX, SPEED_100,
+				       ETHTOOL_LINK_MODE_100baseT_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_1000BASE_T, SPEED_1000,
+				       ETHTOOL_LINK_MODE_1000baseT_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_1000BASE_CX_SGMII, SPEED_1000,
+				       ETHTOOL_LINK_MODE_1000baseKX_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_1000BASE_KX, SPEED_1000,
+				       ETHTOOL_LINK_MODE_1000baseKX_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_T, SPEED_10000,
+				       ETHTOOL_LINK_MODE_10000baseT_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_CX4, SPEED_10000,
+				       ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_KX4, SPEED_10000,
+				       ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_KR, SPEED_10000,
+				       ETHTOOL_LINK_MODE_10000baseKR_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_CR, SPEED_10000,
+				       ETHTOOL_LINK_MODE_10000baseKR_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_10GBASE_SR, SPEED_10000,
+				       ETHTOOL_LINK_MODE_10000baseKR_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_20GBASE_KR2, SPEED_20000,
+				       ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT,
+				       ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_40GBASE_CR4, SPEED_40000,
+				       ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_40GBASE_KR4, SPEED_40000,
+				       ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_40GBASE_SR4, SPEED_40000,
+				       ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_56GBASE_KR4, SPEED_56000,
+				       ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_56GBASE_CR4, SPEED_56000,
+				       ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT);
+	MLX4_BUILD_PTYS2ETHTOOL_CONFIG(MLX4_56GBASE_SR4, SPEED_56000,
+				       ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT);
 };
 
-static u32 ptys2ethtool_link_modes(u32 eth_proto, enum ethtool_report report)
+static void ptys2ethtool_update_link_modes(unsigned long *link_modes,
+					   u32 eth_proto,
+					   enum ethtool_report report)
 {
 	int i;
-	u32 link_modes = 0;
-
 	for (i = 0; i < MLX4_LINK_MODES_SZ; i++) {
 		if (eth_proto & MLX4_PROT_MASK(i))
-			link_modes |= ptys2ethtool_map[i][report];
+			bitmap_or(link_modes, link_modes,
+				  ptys2ethtool_link_mode(&ptys2ethtool_map[i],
+							 report),
+				  __ETHTOOL_LINK_MODE_MASK_NBITS);
 	}
-	return link_modes;
 }
 
-static u32 ethtool2ptys_link_modes(u32 link_modes, enum ethtool_report report)
+static u32 ethtool2ptys_link_modes(const unsigned long *link_modes,
+				   enum ethtool_report report)
 {
 	int i;
 	u32 ptys_modes = 0;
 
 	for (i = 0; i < MLX4_LINK_MODES_SZ; i++) {
-		if (ptys2ethtool_map[i][report] & link_modes)
+		if (bitmap_intersects(
+			    ptys2ethtool_link_mode(&ptys2ethtool_map[i],
+						   report),
+			    link_modes,
+			    __ETHTOOL_LINK_MODE_MASK_NBITS))
 			ptys_modes |= 1 << i;
 	}
 	return ptys_modes;
@@ -702,14 +687,14 @@ static u32 speed2ptys_link_modes(u32 speed)
 	u32 ptys_modes = 0;
 
 	for (i = 0; i < MLX4_LINK_MODES_SZ; i++) {
-		if (ptys2ethtool_map[i][SPEED] == speed)
+		if (ptys2ethtool_map[i].speed == speed)
 			ptys_modes |= 1 << i;
 	}
 	return ptys_modes;
 }
 
-static int ethtool_get_ptys_settings(struct net_device *dev,
-				     struct ethtool_cmd *cmd)
+static int ethtool_get_ptys_ksettings(struct net_device *dev,
+				      struct ethtool_ksettings *ksettings)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_ptys_reg ptys_reg;
@@ -737,79 +722,93 @@ static int ethtool_get_ptys_settings(struct net_device *dev,
 	en_dbg(DRV, priv, "ptys_reg.eth_proto_lp_adv %x\n",
 	       be32_to_cpu(ptys_reg.eth_proto_lp_adv));
 
-	cmd->supported = 0;
-	cmd->advertising = 0;
+	/* reset supported/advertising masks */
+	ethtool_ksettings_zero_link_mode(ksettings, supported);
+	ethtool_ksettings_zero_link_mode(ksettings, advertising);
 
-	cmd->supported |= ptys_get_supported_port(&ptys_reg);
+	ptys2ethtool_update_supported_port(ksettings->link_modes.supported,
+					   &ptys_reg);
 
 	eth_proto = be32_to_cpu(ptys_reg.eth_proto_cap);
-	cmd->supported |= ptys2ethtool_link_modes(eth_proto, SUPPORTED);
+	ptys2ethtool_update_link_modes(ksettings->link_modes.supported,
+				       eth_proto, SUPPORTED);
 
 	eth_proto = be32_to_cpu(ptys_reg.eth_proto_admin);
-	cmd->advertising |= ptys2ethtool_link_modes(eth_proto, ADVERTISED);
+	ptys2ethtool_update_link_modes(ksettings->link_modes.advertising,
+				       eth_proto, ADVERTISED);
 
-	cmd->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
-	cmd->advertising |= (priv->prof->tx_pause) ? ADVERTISED_Pause : 0;
+	ethtool_ksettings_add_link_mode(ksettings, supported, Pause);
+	ethtool_ksettings_add_link_mode(ksettings, supported, Asym_Pause);
 
-	cmd->advertising |= (priv->prof->tx_pause ^ priv->prof->rx_pause) ?
-		ADVERTISED_Asym_Pause : 0;
+	if (priv->prof->tx_pause)
+		ethtool_ksettings_add_link_mode(ksettings, advertising,
+						Pause);
+	if (priv->prof->tx_pause ^ priv->prof->rx_pause)
+		ethtool_ksettings_add_link_mode(ksettings, advertising,
+						Asym_Pause);
 
-	cmd->port = ptys_get_active_port(&ptys_reg);
-	cmd->transceiver = (SUPPORTED_TP & cmd->supported) ?
-		XCVR_EXTERNAL : XCVR_INTERNAL;
+	ksettings->parent.port = ptys_get_active_port(&ptys_reg);
 
 	if (mlx4_en_autoneg_get(dev)) {
-		cmd->supported |= SUPPORTED_Autoneg;
-		cmd->advertising |= ADVERTISED_Autoneg;
+		ethtool_ksettings_add_link_mode(ksettings, supported,
+						Autoneg);
+		ethtool_ksettings_add_link_mode(ksettings, advertising,
+						Autoneg);
 	}
 
-	cmd->autoneg = (priv->port_state.flags & MLX4_EN_PORT_ANC) ?
+	ksettings->parent.autoneg
+		= (priv->port_state.flags & MLX4_EN_PORT_ANC) ?
 		AUTONEG_ENABLE : AUTONEG_DISABLE;
 
 	eth_proto = be32_to_cpu(ptys_reg.eth_proto_lp_adv);
-	cmd->lp_advertising = ptys2ethtool_link_modes(eth_proto, ADVERTISED);
 
-	cmd->lp_advertising |= (priv->port_state.flags & MLX4_EN_PORT_ANC) ?
-			ADVERTISED_Autoneg : 0;
+	ethtool_ksettings_zero_link_mode(ksettings, lp_advertising);
+	ptys2ethtool_update_link_modes(ksettings->link_modes.lp_advertising,
+				       eth_proto, ADVERTISED);
+	if (priv->port_state.flags & MLX4_EN_PORT_ANC)
+		ethtool_ksettings_add_link_mode(ksettings, lp_advertising,
+						Autoneg);
 
-	cmd->phy_address = 0;
-	cmd->mdio_support = 0;
-	cmd->maxtxpkt = 0;
-	cmd->maxrxpkt = 0;
-	cmd->eth_tp_mdix = ETH_TP_MDI_INVALID;
-	cmd->eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
+	ksettings->parent.phy_address = 0;
+	ksettings->parent.mdio_support = 0;
+	ksettings->parent.eth_tp_mdix = ETH_TP_MDI_INVALID;
+	ksettings->parent.eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
 
 	return ret;
 }
 
-static void ethtool_get_default_settings(struct net_device *dev,
-					 struct ethtool_cmd *cmd)
+static void ethtool_get_default_ksettings(struct net_device *dev,
+					  struct ethtool_ksettings *ksettings)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	int trans_type;
 
-	cmd->autoneg = AUTONEG_DISABLE;
-	cmd->supported = SUPPORTED_10000baseT_Full;
-	cmd->advertising = ADVERTISED_10000baseT_Full;
-	trans_type = priv->port_state.transceiver;
+	ksettings->parent.autoneg = AUTONEG_DISABLE;
 
+	ethtool_ksettings_zero_link_mode(ksettings, supported);
+	ethtool_ksettings_add_link_mode(ksettings, supported,
+					10000baseT_Full);
+
+	ethtool_ksettings_zero_link_mode(ksettings, advertising);
+	ethtool_ksettings_add_link_mode(ksettings, advertising,
+					10000baseT_Full);
+
+	trans_type = priv->port_state.transceiver;
 	if (trans_type > 0 && trans_type <= 0xC) {
-		cmd->port = PORT_FIBRE;
-		cmd->transceiver = XCVR_EXTERNAL;
-		cmd->supported |= SUPPORTED_FIBRE;
-		cmd->advertising |= ADVERTISED_FIBRE;
+		ksettings->parent.port = PORT_FIBRE;
+		ethtool_ksettings_add_link_mode(ksettings, supported, FIBRE);
+		ethtool_ksettings_add_link_mode(ksettings, advertising, FIBRE);
 	} else if (trans_type == 0x80 || trans_type == 0) {
-		cmd->port = PORT_TP;
-		cmd->transceiver = XCVR_INTERNAL;
-		cmd->supported |= SUPPORTED_TP;
-		cmd->advertising |= ADVERTISED_TP;
+		ksettings->parent.port = PORT_TP;
+		ethtool_ksettings_add_link_mode(ksettings, supported, TP);
+		ethtool_ksettings_add_link_mode(ksettings, advertising, TP);
 	} else  {
-		cmd->port = -1;
-		cmd->transceiver = -1;
+		ksettings->parent.port = -1;
 	}
 }
 
-static int mlx4_en_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+static int mlx4_en_get_ksettings(struct net_device *dev,
+				 struct ethtool_ksettings *ksettings)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	int ret = -EINVAL;
@@ -822,16 +821,16 @@ static int mlx4_en_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 	       priv->port_state.flags & MLX4_EN_PORT_ANE);
 
 	if (priv->mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_ETH_PROT_CTRL)
-		ret = ethtool_get_ptys_settings(dev, cmd);
+		ret = ethtool_get_ptys_ksettings(dev, ksettings);
 	if (ret) /* ETH PROT CRTL is not supported or PTYS CMD failed */
-		ethtool_get_default_settings(dev, cmd);
+		ethtool_get_default_ksettings(dev, ksettings);
 
 	if (netif_carrier_ok(dev)) {
-		ethtool_cmd_speed_set(cmd, priv->port_state.link_speed);
-		cmd->duplex = DUPLEX_FULL;
+		ksettings->parent.speed = priv->port_state.link_speed;
+		ksettings->parent.duplex = DUPLEX_FULL;
 	} else {
-		ethtool_cmd_speed_set(cmd, SPEED_UNKNOWN);
-		cmd->duplex = DUPLEX_UNKNOWN;
+		ksettings->parent.speed = SPEED_UNKNOWN;
+		ksettings->parent.duplex = DUPLEX_UNKNOWN;
 	}
 	return 0;
 }
@@ -855,21 +854,28 @@ static __be32 speed_set_ptys_admin(struct mlx4_en_priv *priv, u32 speed,
 	return proto_admin;
 }
 
-static int mlx4_en_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+static int mlx4_en_set_ksettings(struct net_device *dev,
+				 const struct ethtool_ksettings *ksettings)
 {
 	struct mlx4_en_priv *priv = netdev_priv(dev);
 	struct mlx4_ptys_reg ptys_reg;
 	__be32 proto_admin;
 	int ret;
 
-	u32 ptys_adv = ethtool2ptys_link_modes(cmd->advertising, ADVERTISED);
-	int speed = ethtool_cmd_speed(cmd);
+	u32 ptys_adv = ethtool2ptys_link_modes(
+		ksettings->link_modes.advertising, ADVERTISED);
+	const int speed = ksettings->parent.speed;
 
-	en_dbg(DRV, priv, "Set Speed=%d adv=0x%x autoneg=%d duplex=%d\n",
-	       speed, cmd->advertising, cmd->autoneg, cmd->duplex);
+	if (en_dbg_enabled(DRV, priv)) {
+		en_dbg(DRV, priv,
+		       "Set Speed=%d adv={%*pbl} autoneg=%d duplex=%d\n",
+		       speed, __ETHTOOL_LINK_MODE_MASK_NBITS,
+		       ksettings->link_modes.advertising,
+		       ksettings->parent.autoneg, ksettings->parent.duplex);
+	}
 
 	if (!(priv->mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_ETH_PROT_CTRL) ||
-	    (cmd->duplex == DUPLEX_HALF))
+	    (ksettings->parent.duplex == DUPLEX_HALF))
 		return -EINVAL;
 
 	memset(&ptys_reg, 0, sizeof(ptys_reg));
@@ -883,7 +889,7 @@ static int mlx4_en_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 		return 0;
 	}
 
-	proto_admin = cmd->autoneg == AUTONEG_ENABLE ?
+	proto_admin = ksettings->parent.autoneg == AUTONEG_ENABLE ?
 		cpu_to_be32(ptys_adv) :
 		speed_set_ptys_admin(priv, speed,
 				     ptys_reg.eth_proto_cap);
@@ -1982,8 +1988,8 @@ static int mlx4_en_set_phys_id(struct net_device *dev,
 
 const struct ethtool_ops mlx4_en_ethtool_ops = {
 	.get_drvinfo = mlx4_en_get_drvinfo,
-	.get_settings = mlx4_en_get_settings,
-	.set_settings = mlx4_en_set_settings,
+	.get_ksettings = mlx4_en_get_ksettings,
+	.set_ksettings = mlx4_en_set_ksettings,
 	.get_link = ethtool_op_get_link,
 	.get_strings = mlx4_en_get_strings,
 	.get_sset_count = mlx4_en_get_sset_count,
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_main.c b/drivers/net/ethernet/mellanox/mlx4/en_main.c
index 005f910..98913bb 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_main.c
@@ -389,6 +389,7 @@ static void mlx4_en_verify_params(void)
 static int __init mlx4_en_init(void)
 {
 	mlx4_en_verify_params();
+	mlx4_en_init_ptys2ethtool_map();
 
 	return mlx4_register_interface(&mlx4_en_interface);
 }
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index b04054d..2d8af43 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -607,6 +607,7 @@ static inline struct mlx4_cqe *mlx4_en_get_cqe(void *buf, int idx, int cqe_sz)
 
 #define MLX4_EN_WOL_DO_MODIFY (1ULL << 63)
 
+void mlx4_en_init_ptys2ethtool_map(void);
 void mlx4_en_update_loopback_state(struct net_device *dev,
 				   netdev_features_t features);
 
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1291571 — [PATCH net-next v5 17/19] net: ethtool: remove unused __ethtool_get_settings

FromDavid Decotigny <ddecotig@gmail.com>
Date2015-12-14 22:10 +0100
Subject[PATCH net-next v5 17/19] net: ethtool: remove unused __ethtool_get_settings
Message-ID<qFIxB-34I-49@gated-at.bofh.it>
In reply to#1291569
From: David Decotigny <decot@googlers.com>

replaced by __ethtool_get_ksettings.

Signed-off-by: David Decotigny <decot@googlers.com>
---
 include/linux/ethtool.h |  4 ----
 net/core/ethtool.c      | 45 ++++++++++++++-------------------------------
 2 files changed, 14 insertions(+), 35 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 6077cbb..05d4f0e 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -148,10 +148,6 @@ struct ethtool_ksettings {
 extern int __ethtool_get_ksettings(struct net_device *dev,
 				   struct ethtool_ksettings *ksettings);
 
-/* DEPRECATED, use __ethtool_get_ksettings */
-extern int __ethtool_get_settings(struct net_device *dev,
-				  struct ethtool_cmd *cmd);
-
 /**
  * struct ethtool_ops - optional netdev operations
  * @get_settings: DEPRECATED, use %get_ksettings/%set_ksettings
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4865031..84dca87 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -495,7 +495,12 @@ int __ethtool_get_ksettings(struct net_device *dev,
 	 * legacy %ethtool_cmd API, unless it's not supported either.
 	 * TODO: remove when ethtool_ops::get_settings disappears internally
 	 */
-	err = __ethtool_get_settings(dev, &cmd);
+	if (!dev->ethtool_ops->get_settings)
+		return -EOPNOTSUPP;
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.cmd = ETHTOOL_GSET;
+	err = dev->ethtool_ops->get_settings(dev, &cmd);
 	if (err < 0)
 		return err;
 
@@ -652,30 +657,6 @@ static int ethtool_set_ksettings(struct net_device *dev, void __user *useraddr)
 	return dev->ethtool_ops->set_ksettings(dev, &ksettings);
 }
 
-/* Internal kernel helper to query a device ethtool_cmd settings.
- *
- * Note about transition to ethtool_settings API: We do not need (or
- * want) this function to support "dev" instances that implement the
- * ethtool_settings API as we will update the drivers calling this
- * function to call __ethtool_get_ksettings instead, before the first
- * drivers implement ethtool_ops::get_ksettings.
- *
- * TODO 1: at least make this function static when no driver is using it
- * TODO 2: remove when ethtool_ops::get_settings disappears internally
- */
-int __ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
-{
-	ASSERT_RTNL();
-
-	if (!dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
-
-	memset(cmd, 0, sizeof(struct ethtool_cmd));
-	cmd->cmd = ETHTOOL_GSET;
-	return dev->ethtool_ops->get_settings(dev, cmd);
-}
-EXPORT_SYMBOL(__ethtool_get_settings);
-
 static void
 warn_incomplete_ethtool_legacy_settings_conversion(const char *details)
 {
@@ -717,16 +698,18 @@ static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 		/* send a sensible cmd tag back to user */
 		cmd.cmd = ETHTOOL_GSET;
 	} else {
-		int err;
-		/* TODO: return -EOPNOTSUPP when
-		 * ethtool_ops::get_settings disappears internally
-		 */
-
 		/* driver doesn't support %ethtool_ksettings
 		 * API. revert to legacy %ethtool_cmd API, unless it's
 		 * not supported either.
 		 */
-		err = __ethtool_get_settings(dev, &cmd);
+		int err;
+
+		if (!dev->ethtool_ops->get_settings)
+			return -EOPNOTSUPP;
+
+		memset(&cmd, 0, sizeof(cmd));
+		cmd.cmd = ETHTOOL_GSET;
+		err = dev->ethtool_ops->get_settings(dev, &cmd);
 		if (err < 0)
 			return err;
 	}
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1291576 — [PATCH net-next v5 02/19] test_bitmap: unit tests for lib/bitmap.c

FromDavid Decotigny <ddecotig@gmail.com>
Date2015-12-14 22:20 +0100
Subject[PATCH net-next v5 02/19] test_bitmap: unit tests for lib/bitmap.c
Message-ID<qFIHg-382-9@gated-at.bofh.it>
In reply to#1291569
From: David Decotigny <decot@googlers.com>

This is mainly testing bitmap construction and conversion to/from u32[]
for now.

Tested:
  qemu i386, x86_64, ppc, ppc64 BE and LE, ARM.

Signed-off-by: David Decotigny <decot@googlers.com>
---
 lib/Kconfig.debug                     |   8 +
 lib/Makefile                          |   1 +
 lib/test_bitmap.c                     | 343 ++++++++++++++++++++++++++++++++++
 tools/testing/selftests/lib/Makefile  |   2 +-
 tools/testing/selftests/lib/bitmap.sh |  10 +
 5 files changed, 363 insertions(+), 1 deletion(-)
 create mode 100644 lib/test_bitmap.c
 create mode 100644 tools/testing/selftests/lib/bitmap.sh

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0d76ecc..3d25bdf 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1721,6 +1721,14 @@ config TEST_KSTRTOX
 config TEST_PRINTF
 	tristate "Test printf() family of functions at runtime"
 
+config TEST_BITMAP
+	tristate "Test bitmap_*() family of functions at runtime"
+	default n
+	help
+	  Enable this option to test the bitmap functions at boot.
+
+	  If unsure, say N.
+
 config TEST_RHASHTABLE
 	tristate "Perform selftest on resizable hash table"
 	default n
diff --git a/lib/Makefile b/lib/Makefile
index 180dd4d..ba6b7fe 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
 obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
 obj-$(CONFIG_TEST_PRINTF) += test_printf.o
+obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
 
 ifeq ($(CONFIG_DEBUG_KOBJECT),y)
 CFLAGS_kobject.o += -DDEBUG
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
new file mode 100644
index 0000000..33e572a
--- /dev/null
+++ b/lib/test_bitmap.c
@@ -0,0 +1,343 @@
+/*
+ * Test cases for printf facility.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitmap.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+static unsigned total_tests __initdata;
+static unsigned failed_tests __initdata;
+
+static char pbl_buffer[PAGE_SIZE] __initdata;
+
+
+static bool __init
+__check_eq_bitmap(const unsigned long *exp_bmap, unsigned int exp_nbits,
+		  const unsigned long *bmap, unsigned int nbits)
+{
+	if (exp_nbits != nbits) {
+		pr_warn("bitmap length mismatch: expected %u, got %u\n",
+			exp_nbits, nbits);
+		return false;
+	}
+
+	if (!bitmap_equal(exp_bmap, bmap, nbits)) {
+		pr_warn("bitmaps contents differ: expected \"%*pbl\", got \"%*pbl\"\n",
+			exp_nbits, exp_bmap, nbits, bmap);
+		return false;
+	}
+	return true;
+}
+
+static int __init
+expect_eq_bitmap(const unsigned long *exp_bmap, unsigned int exp_nbits,
+	       const unsigned long *bmap, unsigned int nbits)
+{
+	total_tests++;
+	if (!__check_eq_bitmap(exp_bmap, exp_nbits, bmap, nbits)) {
+		failed_tests++;
+		return 1;
+	}
+	return 0;
+}
+
+static bool __init
+__check_eq_pbl(const char *expected_pbl,
+	       const unsigned long *bitmap, unsigned int nbits)
+{
+	snprintf(pbl_buffer, sizeof(pbl_buffer), "%*pbl", nbits, bitmap);
+	if (strcmp(expected_pbl, pbl_buffer)) {
+		pr_warn("expected \"%s\", got \"%s\"\n",
+			expected_pbl, pbl_buffer);
+		return false;
+	}
+	return true;
+}
+
+static int __init
+expect_eq_pbl(const char *expected_pbl,
+	    const unsigned long *bitmap, unsigned int nbits)
+{
+	total_tests++;
+	if (!__check_eq_pbl(expected_pbl, bitmap, nbits)) {
+		failed_tests++;
+		return 1;
+	}
+	return 0;
+}
+
+static bool __init
+__check_eq_u32_array(const u32 *exp_arr, unsigned int exp_len,
+		     const u32 *arr, unsigned int len)
+{
+	if (exp_len != len) {
+		pr_warn("array length differ: expected %u, got %u\n",
+			exp_len, len);
+		return false;
+	}
+
+	if (memcmp(exp_arr, arr, len*sizeof(*arr))) {
+		pr_warn("array contents differ\n");
+		print_hex_dump(KERN_WARNING, "  exp:  ", DUMP_PREFIX_OFFSET,
+			       32, 4, exp_arr, exp_len*sizeof(*exp_arr), false);
+		print_hex_dump(KERN_WARNING, "  got:  ", DUMP_PREFIX_OFFSET,
+			       32, 4, arr, len*sizeof(*arr), false);
+		return false;
+	}
+
+	return true;
+}
+
+static int __init
+expect_eq_u32_array(const u32 *exp_arr, unsigned int exp_len,
+		  const u32 *arr, unsigned int len)
+{
+	total_tests++;
+	if (!__check_eq_u32_array(exp_arr, exp_len, arr, len)) {
+		failed_tests++;
+		return 1;
+	}
+	return 0;
+}
+
+static void __init test_zero_fill_copy(void)
+{
+	DECLARE_BITMAP(bmap1, 1024);
+	DECLARE_BITMAP(bmap2, 1024);
+
+	bitmap_zero(bmap1, 1024);
+	bitmap_zero(bmap2, 1024);
+
+	/* single-word bitmaps */
+	expect_eq_pbl("", bmap1, 23);
+
+	bitmap_fill(bmap1, 19);
+	expect_eq_pbl("0-18", bmap1, 1024);
+
+	bitmap_copy(bmap2, bmap1, 23);
+	expect_eq_pbl("0-18", bmap2, 1024);
+
+	bitmap_fill(bmap2, 23);
+	expect_eq_pbl("0-22", bmap2, 1024);
+
+	bitmap_copy(bmap2, bmap1, 23);
+	expect_eq_pbl("0-18", bmap2, 1024);
+
+	bitmap_zero(bmap1, 23);
+	expect_eq_pbl("", bmap1, 1024);
+
+	/* multi-word bitmaps */
+	bitmap_zero(bmap1, 1024);
+	expect_eq_pbl("", bmap1, 1024);
+
+	bitmap_fill(bmap1, 109);
+	expect_eq_pbl("0-108", bmap1, 1024);
+
+	bitmap_copy(bmap2, bmap1, 1024);
+	expect_eq_pbl("0-108", bmap2, 1024);
+
+	bitmap_fill(bmap2, 1024);
+	expect_eq_pbl("0-1023", bmap2, 1024);
+
+	bitmap_copy(bmap2, bmap1, 1024);
+	expect_eq_pbl("0-108", bmap2, 1024);
+
+	/* the following tests assume a 32- or 64-bit arch (even 128b
+	 * if we care)
+	 */
+
+	bitmap_fill(bmap2, 1024);
+	bitmap_copy(bmap2, bmap1, 109);  /* ... but 0-padded til word length */
+	expect_eq_pbl("0-108,128-1023", bmap2, 1024);
+
+	bitmap_fill(bmap2, 1024);
+	bitmap_copy(bmap2, bmap1, 97);  /* ... but aligned on word length */
+	expect_eq_pbl("0-108,128-1023", bmap2, 1024);
+
+	bitmap_zero(bmap2, 97);  /* ... but 0-padded til word length */
+	expect_eq_pbl("128-1023", bmap2, 1024);
+}
+
+static void __init test_bitmap_u32_array_conversions(void)
+{
+	DECLARE_BITMAP(bmap1, 1024);
+	DECLARE_BITMAP(bmap2, 1024);
+	u32 exp_arr[32], arr[32];
+	unsigned nbits;
+
+	for (nbits = 0 ; nbits < 257 ; ++nbits) {
+		const unsigned int used_u32s = DIV_ROUND_UP(nbits, 32);
+		unsigned int i;
+
+		bitmap_zero(bmap1, nbits);
+		bitmap_set(bmap1, nbits, 1024 - nbits);  /* garbage */
+
+		memset(arr, 0xff, sizeof(arr));
+		bitmap_to_u32array(arr, used_u32s, bmap1, nbits);
+
+		memset(exp_arr, 0xff, sizeof(exp_arr));
+		memset(exp_arr, 0, used_u32s*sizeof(*exp_arr));
+		expect_eq_u32_array(exp_arr, 32, arr, 32);
+
+		bitmap_fill(bmap2, 1024);
+		bitmap_from_u32array(bmap2, nbits, arr, used_u32s);
+		expect_eq_bitmap(bmap1, 1024, bmap2, 1024);
+
+		for (i = 0 ; i < nbits ; ++i) {
+			/*
+			 * test conversion bitmap -> u32[]
+			 */
+
+			bitmap_zero(bmap1, 1024);
+			__set_bit(i, bmap1);
+			bitmap_set(bmap1, nbits, 1024 - nbits);  /* garbage */
+
+			memset(arr, 0xff, sizeof(arr));
+			bitmap_to_u32array(arr, used_u32s, bmap1, nbits);
+
+			/* 1st used u32 words contain expected bit set, the
+			 * remaining words are left unchanged (0xff)
+			 */
+			memset(exp_arr, 0xff, sizeof(exp_arr));
+			memset(exp_arr, 0, used_u32s*sizeof(*exp_arr));
+			exp_arr[i/32] = (1U<<(i%32));
+			expect_eq_u32_array(exp_arr, 32, arr, 32);
+
+
+			/* same, with longer array to fill
+			 */
+			memset(arr, 0xff, sizeof(arr));
+			bitmap_to_u32array(arr, 32, bmap1, nbits);
+
+			/* 1st used u32 words contain expected bit set, the
+			 * remaining words are all 0s
+			 */
+			memset(exp_arr, 0, sizeof(exp_arr));
+			exp_arr[i/32] = (1U<<(i%32));
+			expect_eq_u32_array(exp_arr, 32, arr, 32);
+
+			/*
+			 * test conversion u32[] -> bitmap
+			 */
+
+			/* the 1st nbits of bmap2 are identical to
+			 * bmap1, the remaining bits of bmap2 are left
+			 * unchanged (all 1s)
+			 */
+			bitmap_fill(bmap2, 1024);
+			bitmap_from_u32array(bmap2, nbits, exp_arr, used_u32s);
+
+			expect_eq_bitmap(bmap1, 1024, bmap2, 1024);
+
+			/* same, with more bits to fill
+			 */
+			memset(arr, 0xff, sizeof(arr));  /* garbage */
+			memset(arr, 0, used_u32s*sizeof(u32));
+			arr[i/32] = (1U<<(i%32));
+
+			bitmap_fill(bmap2, 1024);
+			bitmap_from_u32array(bmap2, 1024, arr, used_u32s);
+
+			/* the 1st nbits of bmap2 are identical to
+			 * bmap1, the remaining bits of bmap2 are cleared
+			 */
+			bitmap_zero(bmap1, 1024);
+			__set_bit(i, bmap1);
+			expect_eq_bitmap(bmap1, 1024, bmap2, 1024);
+
+
+			/*
+			 * test short conversion bitmap -> u32[] (1
+			 * word too short)
+			 */
+			if (used_u32s > 1) {
+				bitmap_zero(bmap1, 1024);
+				__set_bit(i, bmap1);
+				bitmap_set(bmap1, nbits,
+					   1024 - nbits);  /* garbage */
+				memset(arr, 0xff, sizeof(arr));
+
+				bitmap_to_u32array(arr, used_u32s - 1,
+						   bmap1, nbits);
+
+				/* 1st used u32 words contain expected
+				 * bit set, the remaining words are
+				 * left unchanged (0xff)
+				 */
+				memset(exp_arr, 0xff, sizeof(exp_arr));
+				memset(exp_arr, 0,
+				       (used_u32s-1)*sizeof(*exp_arr));
+				if ((i/32) < (used_u32s - 1))
+					exp_arr[i/32] = (1U<<(i%32));
+				expect_eq_u32_array(exp_arr, 32, arr, 32);
+			}
+
+			/*
+			 * test short conversion u32[] -> bitmap (3
+			 * bits too short)
+			 */
+			if (nbits > 3) {
+				memset(arr, 0xff, sizeof(arr));  /* garbage */
+				memset(arr, 0, used_u32s*sizeof(*arr));
+				arr[i/32] = (1U<<(i%32));
+
+				bitmap_zero(bmap1, 1024);
+				bitmap_from_u32array(bmap1, nbits - 3,
+						     arr, used_u32s);
+
+				/* we are expecting the bit < nbits -
+				 * 3 (none otherwise), and the rest of
+				 * bmap1 unchanged (0-filled)
+				 */
+				bitmap_zero(bmap2, 1024);
+				if (i < nbits - 3)
+					__set_bit(i, bmap2);
+				expect_eq_bitmap(bmap2, 1024, bmap1, 1024);
+
+				/* do the same with bmap1 initially
+				 * 1-filled
+				 */
+
+				bitmap_fill(bmap1, 1024);
+				bitmap_from_u32array(bmap1, nbits - 3,
+						     arr, used_u32s);
+
+				/* we are expecting the bit < nbits -
+				 * 3 (none otherwise), and the rest of
+				 * bmap1 unchanged (1-filled)
+				 */
+				bitmap_zero(bmap2, 1024);
+				if (i < nbits - 3)
+					__set_bit(i, bmap2);
+				bitmap_set(bmap2, nbits-3, 1024 - nbits + 3);
+				expect_eq_bitmap(bmap2, 1024, bmap1, 1024);
+			}
+		}
+	}
+}
+
+static int __init test_bitmap_init(void)
+{
+	test_zero_fill_copy();
+	test_bitmap_u32_array_conversions();
+
+	if (failed_tests == 0)
+		pr_info("all %u tests passed\n", total_tests);
+	else
+		pr_warn("failed %u out of %u tests\n",
+			failed_tests, total_tests);
+
+	return failed_tests ? -EINVAL : 0;
+}
+
+module_init(test_bitmap_init);
+
+MODULE_AUTHOR("david decotigny <david.decotigny@googlers.com>");
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile
index 47147b9..0836006 100644
--- a/tools/testing/selftests/lib/Makefile
+++ b/tools/testing/selftests/lib/Makefile
@@ -3,6 +3,6 @@
 # No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
 all:
 
-TEST_PROGS := printf.sh
+TEST_PROGS := printf.sh bitmap.sh
 
 include ../lib.mk
diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
new file mode 100644
index 0000000..2da187b
--- /dev/null
+++ b/tools/testing/selftests/lib/bitmap.sh
@@ -0,0 +1,10 @@
+#!/bin/sh
+# Runs bitmap infrastructure tests using test_bitmap kernel module
+
+if /sbin/modprobe -q test_bitmap; then
+	/sbin/modprobe -q -r test_bitmap
+	echo "bitmap: ok"
+else
+	echo "bitmap: [FAIL]"
+	exit 1
+fi
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1291578 — [PATCH net-next v5 03/19] net: usnic: remove unused call to ethtool_ops::get_settings

FromDavid Decotigny <ddecotig@gmail.com>
Date2015-12-14 22:20 +0100
Subject[PATCH net-next v5 03/19] net: usnic: remove unused call to ethtool_ops::get_settings
Message-ID<qFIHg-382-29@gated-at.bofh.it>
In reply to#1291569
From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
index f8e3211..5b60579 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
@@ -269,7 +269,6 @@ int usnic_ib_query_device(struct ib_device *ibdev,
 	struct usnic_ib_dev *us_ibdev = to_usdev(ibdev);
 	union ib_gid gid;
 	struct ethtool_drvinfo info;
-	struct ethtool_cmd cmd;
 	int qp_per_vf;
 
 	usnic_dbg("\n");
@@ -278,7 +277,6 @@ int usnic_ib_query_device(struct ib_device *ibdev,
 
 	mutex_lock(&us_ibdev->usdev_lock);
 	us_ibdev->netdev->ethtool_ops->get_drvinfo(us_ibdev->netdev, &info);
-	us_ibdev->netdev->ethtool_ops->get_settings(us_ibdev->netdev, &cmd);
 	memset(props, 0, sizeof(*props));
 	usnic_mac_ip_to_gid(us_ibdev->ufdev->mac, us_ibdev->ufdev->inaddr,
 			&gid.raw[0]);
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1291579 — [PATCH net-next v5 07/19] net: usnic: use __ethtool_get_ksettings

FromDavid Decotigny <ddecotig@gmail.com>
Date2015-12-14 22:20 +0100
Subject[PATCH net-next v5 07/19] net: usnic: use __ethtool_get_ksettings
Message-ID<qFIHg-382-15@gated-at.bofh.it>
In reply to#1291569
From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
index e082170..e0d12d4 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_verbs.c
@@ -324,12 +324,12 @@ int usnic_ib_query_port(struct ib_device *ibdev, u8 port,
 				struct ib_port_attr *props)
 {
 	struct usnic_ib_dev *us_ibdev = to_usdev(ibdev);
-	struct ethtool_cmd cmd;
+	struct ethtool_ksettings cmd;
 
 	usnic_dbg("\n");
 
 	mutex_lock(&us_ibdev->usdev_lock);
-	__ethtool_get_settings(us_ibdev->netdev, &cmd);
+	__ethtool_get_ksettings(us_ibdev->netdev, &cmd);
 	memset(props, 0, sizeof(*props));
 
 	props->lid = 0;
@@ -353,8 +353,8 @@ int usnic_ib_query_port(struct ib_device *ibdev, u8 port,
 	props->pkey_tbl_len = 1;
 	props->bad_pkey_cntr = 0;
 	props->qkey_viol_cntr = 0;
-	eth_speed_to_ib_speed(cmd.speed, &props->active_speed,
-				&props->active_width);
+	eth_speed_to_ib_speed(cmd.parent.speed, &props->active_speed,
+			      &props->active_width);
 	props->max_mtu = IB_MTU_4096;
 	props->active_mtu = iboe_get_mtu(us_ibdev->ufdev->mtu);
 	/* Userspace will adjust for hdrs */
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1291580 — [PATCH net-next v5 06/19] tx4939: use __ethtool_get_ksettings

FromDavid Decotigny <ddecotig@gmail.com>
Date2015-12-14 22:20 +0100
Subject[PATCH net-next v5 06/19] tx4939: use __ethtool_get_ksettings
Message-ID<qFIHg-382-17@gated-at.bofh.it>
In reply to#1291569
From: David Decotigny <decot@googlers.com>

Signed-off-by: David Decotigny <decot@googlers.com>
---
 arch/mips/txx9/generic/setup_tx4939.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/mips/txx9/generic/setup_tx4939.c b/arch/mips/txx9/generic/setup_tx4939.c
index e3733cd..4a3ebf6 100644
--- a/arch/mips/txx9/generic/setup_tx4939.c
+++ b/arch/mips/txx9/generic/setup_tx4939.c
@@ -320,11 +320,12 @@ void __init tx4939_sio_init(unsigned int sclk, unsigned int cts_mask)
 #if IS_ENABLED(CONFIG_TC35815)
 static u32 tx4939_get_eth_speed(struct net_device *dev)
 {
-	struct ethtool_cmd cmd;
-	if (__ethtool_get_settings(dev, &cmd))
+	struct ethtool_ksettings cmd;
+
+	if (__ethtool_get_ksettings(dev, &cmd))
 		return 100;	/* default 100Mbps */
 
-	return ethtool_cmd_speed(&cmd);
+	return cmd.parent.speed;
 }
 
 static int tx4939_netdev_event(struct notifier_block *this,
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293088 — Re: [PATCH net-next v5 05/19] net: ethtool: add new ETHTOOL_GSETTINGS/SSETTINGS API

FromDavid Miller <davem@davemloft.net>
Date2015-12-16 17:40 +0100
SubjectRe: [PATCH net-next v5 05/19] net: ethtool: add new ETHTOOL_GSETTINGS/SSETTINGS API
Message-ID<qGnho-4bq-19@gated-at.bofh.it>
In reply to#1291569
From: David Decotigny <ddecotig@gmail.com>
Date: Mon, 14 Dec 2015 13:03:52 -0800

> +static int ethtool_get_ksettings(struct net_device *dev, void __user *useraddr)
> +{
 ...
> +	if (__ETHTOOL_LINK_MODE_MASK_NU32
> +	    != ksettings.parent.link_mode_masks_nwords) {
> +		/* wrong link mode nbits requested */
> +		memset(&ksettings, 0, sizeof(ksettings));
> +		/* keep cmd field reset to 0 */
> +		/* send back number of words required as negative val */
> +		compiletime_assert(__ETHTOOL_LINK_MODE_MASK_NU32 <= S8_MAX,
> +				   "need too many bits for link modes!");
> +		ksettings.parent.link_mode_masks_nwords
> +			= -((s8)__ETHTOOL_LINK_MODE_MASK_NU32);

I'm trying to understand how this can work.

Supposedly, the link_mode_masks_nwords field is there so that we can
add new link modes yet still work with tools built against any
particular link mode list in the UAPI header files.

But here you're forcing the value of link_mode_masks_nwords and then
copying that amount back to userspace.  If the user allocated less
space than the the link mode list in the kernel supports, we will
overwrite past the end of the user's usettings object.

You cannot unconditionally copy sizeof(usettings) back to the user,
as store_ksettings_for_user() will do.

I think you have to truncate here, copying only the array elements the
user's structure actually has space for.  That's the only way this can
work.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1293421 — Re: [PATCH net-next v5 05/19] net: ethtool: add new ETHTOOL_GSETTINGS/SSETTINGS API

FromDavid Decotigny <ddecotig@gmail.com>
Date2015-12-17 01:30 +0100
SubjectRe: [PATCH net-next v5 05/19] net: ethtool: add new ETHTOOL_GSETTINGS/SSETTINGS API
Message-ID<qGuCe-se-13@gated-at.bofh.it>
In reply to#1293088
Thanks David: you are right, we should copy back sizeof(struct
ethtool_settings) in that case and not sizeof(usettings). Sorry about
that, will fix for v6.

a few questions before sending update:
 - is this handshake reasonable? or should we have an ethtool cmd
dedicated to this kind of handshake, or should we hardcode bitmap
length once and for all (ie. 128 instead of 32 today)?
 - is it ok to have the u32[] api part of bitmap.h? I was assuming it
could be used for other ioctl/syscalls outside ethtool... but maybe
this is being too pretentious and we could keep this internal to
net/core?
 - struct inheritance is used to have the link mode bitmaps
piggybacked at end the public struct ethtool_settings. hence this
"parent" field. I'm not super proud of this, but I find relying on the
compiler more comfortable. could revise my position with a
constructor+accessors macros if you think it's preferred.


On Wed, Dec 16, 2015 at 8:38 AM, David Miller <davem@davemloft.net> wrote:
> From: David Decotigny <ddecotig@gmail.com>
> Date: Mon, 14 Dec 2015 13:03:52 -0800
>
>> +static int ethtool_get_ksettings(struct net_device *dev, void __user *useraddr)
>> +{
>  ...
>> +     if (__ETHTOOL_LINK_MODE_MASK_NU32
>> +         != ksettings.parent.link_mode_masks_nwords) {
>> +             /* wrong link mode nbits requested */
>> +             memset(&ksettings, 0, sizeof(ksettings));
>> +             /* keep cmd field reset to 0 */
>> +             /* send back number of words required as negative val */
>> +             compiletime_assert(__ETHTOOL_LINK_MODE_MASK_NU32 <= S8_MAX,
>> +                                "need too many bits for link modes!");
>> +             ksettings.parent.link_mode_masks_nwords
>> +                     = -((s8)__ETHTOOL_LINK_MODE_MASK_NU32);
>
> I'm trying to understand how this can work.
>
> Supposedly, the link_mode_masks_nwords field is there so that we can
> add new link modes yet still work with tools built against any
> particular link mode list in the UAPI header files.
>
> But here you're forcing the value of link_mode_masks_nwords and then
> copying that amount back to userspace.  If the user allocated less
> space than the the link mode list in the kernel supports, we will
> overwrite past the end of the user's usettings object.
>
> You cannot unconditionally copy sizeof(usettings) back to the user,
> as store_ksettings_for_user() will do.
>
> I think you have to truncate here, copying only the array elements the
> user's structure actually has space for.  That's the only way this can
> work.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web