Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 03/21] net: dsa: mv88e6xxx: factorize PHY indirect access
From: Vivien Didelot @ 2016-05-06 21:57 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot
In-Reply-To: <1462571843-14397-1-git-send-email-vivien.didelot@savoirfairelinux.com>

Some switch has dedicated SMI PHY Command and Data registers, used to
indirectly access the PHYs, instead of direct access.

Identify these switch models and make mv88e6xxx_phy_{read,write} generic
enough to support every models.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6171.c |  4 ++--
 drivers/net/dsa/mv88e6352.c |  4 ++--
 drivers/net/dsa/mv88e6xxx.c | 37 ++++---------------------------------
 drivers/net/dsa/mv88e6xxx.h | 22 +++++++++++++---------
 4 files changed, 21 insertions(+), 46 deletions(-)

diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index e64cbee..b190647 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -124,8 +124,8 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 	.probe			= mv88e6171_drv_probe,
 	.setup			= mv88e6171_setup,
 	.set_addr		= mv88e6xxx_set_addr_indirect,
-	.phy_read		= mv88e6xxx_phy_read_indirect,
-	.phy_write		= mv88e6xxx_phy_write_indirect,
+	.phy_read		= mv88e6xxx_phy_read,
+	.phy_write		= mv88e6xxx_phy_write,
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index c61f0f4..6fa7c02 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -344,8 +344,8 @@ struct dsa_switch_driver mv88e6352_switch_driver = {
 	.probe			= mv88e6352_drv_probe,
 	.setup			= mv88e6352_setup,
 	.set_addr		= mv88e6xxx_set_addr_indirect,
-	.phy_read		= mv88e6xxx_phy_read_indirect,
-	.phy_write		= mv88e6xxx_phy_write_indirect,
+	.phy_read		= mv88e6xxx_phy_read,
+	.phy_write		= mv88e6xxx_phy_write,
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index a28b46c..2c8c5e1 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -2887,6 +2887,8 @@ mv88e6xxx_phy_read(struct dsa_switch *ds, int port, int regnum)
 
 	if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_PPU))
 		ret = mv88e6xxx_phy_read_ppu(ps, addr, regnum);
+	else if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_SMI_PHY))
+		ret = _mv88e6xxx_phy_read_indirect(ps, addr, regnum);
 	else
 		ret = _mv88e6xxx_phy_read(ps, addr, regnum);
 
@@ -2908,6 +2910,8 @@ mv88e6xxx_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
 
 	if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_PPU))
 		ret = mv88e6xxx_phy_write_ppu(ps, addr, regnum, val);
+	else if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_SMI_PHY))
+		ret = _mv88e6xxx_phy_write_indirect(ps, addr, regnum, val);
 	else
 		ret = _mv88e6xxx_phy_write(ps, addr, regnum, val);
 
@@ -2915,39 +2919,6 @@ mv88e6xxx_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
 	return ret;
 }
 
-int
-mv88e6xxx_phy_read_indirect(struct dsa_switch *ds, int port, int regnum)
-{
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	int addr = mv88e6xxx_port_to_phy_addr(ps, port);
-	int ret;
-
-	if (addr < 0)
-		return 0xffff;
-
-	mutex_lock(&ps->smi_mutex);
-	ret = _mv88e6xxx_phy_read_indirect(ps, addr, regnum);
-	mutex_unlock(&ps->smi_mutex);
-	return ret;
-}
-
-int
-mv88e6xxx_phy_write_indirect(struct dsa_switch *ds, int port, int regnum,
-			     u16 val)
-{
-	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	int addr = mv88e6xxx_port_to_phy_addr(ps, port);
-	int ret;
-
-	if (addr < 0)
-		return addr;
-
-	mutex_lock(&ps->smi_mutex);
-	ret = _mv88e6xxx_phy_write_indirect(ps, addr, regnum, val);
-	mutex_unlock(&ps->smi_mutex);
-	return ret;
-}
-
 #ifdef CONFIG_NET_DSA_HWMON
 
 static int mv88e61xx_get_temp(struct dsa_switch *ds, int *temp)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 52ca24e..5972571 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -355,10 +355,17 @@ enum mv88e6xxx_cap {
 	 * See GLOBAL_CONTROL_PPU_ENABLE and GLOBAL_STATUS_PPU_POLLING.
 	 */
 	MV88E6XXX_CAP_PPU,
+
+	/* SMI PHY Command and Data registers.
+	 * This requires an indirect access to PHY registers through
+	 * GLOBAL2_SMI_OP, otherwise direct access to PHY registers is done.
+	 */
+	MV88E6XXX_CAP_SMI_PHY,
 };
 
 /* Bitmask of capabilities */
 #define MV88E6XXX_FLAG_PPU		BIT(MV88E6XXX_CAP_PPU)
+#define MV88E6XXX_FLAG_SMI_PHY		BIT(MV88E6XXX_CAP_SMI_PHY)
 
 #define MV88E6XXX_FLAGS_FAMILY_6095	\
 	MV88E6XXX_FLAG_PPU
@@ -371,11 +378,14 @@ enum mv88e6xxx_cap {
 #define MV88E6XXX_FLAGS_FAMILY_6185	\
 	MV88E6XXX_FLAG_PPU
 
-#define MV88E6XXX_FLAGS_FAMILY_6320	0
+#define MV88E6XXX_FLAGS_FAMILY_6320	\
+	MV88E6XXX_FLAG_SMI_PHY
 
-#define MV88E6XXX_FLAGS_FAMILY_6351	0
+#define MV88E6XXX_FLAGS_FAMILY_6351	\
+	MV88E6XXX_FLAG_SMI_PHY
 
-#define MV88E6XXX_FLAGS_FAMILY_6352	0
+#define MV88E6XXX_FLAGS_FAMILY_6352	\
+	MV88E6XXX_FLAG_SMI_PHY
 
 struct mv88e6xxx_info {
 	enum mv88e6xxx_family family;
@@ -497,9 +507,6 @@ int mv88e6xxx_set_addr_direct(struct dsa_switch *ds, u8 *addr);
 int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr);
 int mv88e6xxx_phy_read(struct dsa_switch *ds, int port, int regnum);
 int mv88e6xxx_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val);
-int mv88e6xxx_phy_read_indirect(struct dsa_switch *ds, int port, int regnum);
-int mv88e6xxx_phy_write_indirect(struct dsa_switch *ds, int port, int regnum,
-				 u16 val);
 void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, uint8_t *data);
 void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 				 uint64_t *data);
@@ -516,9 +523,6 @@ int mv88e6xxx_set_temp_limit(struct dsa_switch *ds, int temp);
 int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm);
 int mv88e6xxx_eeprom_load_wait(struct dsa_switch *ds);
 int mv88e6xxx_eeprom_busy_wait(struct dsa_switch *ds);
-int mv88e6xxx_phy_read_indirect(struct dsa_switch *ds, int addr, int regnum);
-int mv88e6xxx_phy_write_indirect(struct dsa_switch *ds, int addr, int regnum,
-				 u16 val);
 int mv88e6xxx_get_eee(struct dsa_switch *ds, int port, struct ethtool_eee *e);
 int mv88e6xxx_set_eee(struct dsa_switch *ds, int port,
 		      struct phy_device *phydev, struct ethtool_eee *e);
-- 
2.8.2

^ permalink raw reply related

* [PATCH net-next 01/21] net: dsa: mv88e6xxx: add flags to info
From: Vivien Didelot @ 2016-05-06 21:57 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot
In-Reply-To: <1462571843-14397-1-git-send-email-vivien.didelot@savoirfairelinux.com>

Add a flags bitmap to the info structure in order to identify features
supported or not by the different switch models.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6123.c |  3 +++
 drivers/net/dsa/mv88e6131.c |  4 ++++
 drivers/net/dsa/mv88e6171.c |  4 ++++
 drivers/net/dsa/mv88e6352.c |  6 ++++++
 drivers/net/dsa/mv88e6xxx.h | 21 +++++++++++++++++++++
 5 files changed, 38 insertions(+)

diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 5535a42..ab5885b 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -24,18 +24,21 @@ static const struct mv88e6xxx_info mv88e6123_table[] = {
 		.name = "Marvell 88E6123",
 		.num_databases = 4096,
 		.num_ports = 3,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6161,
 		.family = MV88E6XXX_FAMILY_6165,
 		.name = "Marvell 88E6161",
 		.num_databases = 4096,
 		.num_ports = 6,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6165,
 		.family = MV88E6XXX_FAMILY_6165,
 		.name = "Marvell 88E6165",
 		.num_databases = 4096,
 		.num_ports = 6,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6165,
 	}
 };
 
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 357ab79..d477320 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -24,24 +24,28 @@ static const struct mv88e6xxx_info mv88e6131_table[] = {
 		.name = "Marvell 88E6095/88E6095F",
 		.num_databases = 256,
 		.num_ports = 11,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6095,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6085,
 		.family = MV88E6XXX_FAMILY_6097,
 		.name = "Marvell 88E6085",
 		.num_databases = 4096,
 		.num_ports = 10,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6097,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6131,
 		.family = MV88E6XXX_FAMILY_6185,
 		.name = "Marvell 88E6131",
 		.num_databases = 256,
 		.num_ports = 8,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6185,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6185,
 		.family = MV88E6XXX_FAMILY_6185,
 		.name = "Marvell 88E6185",
 		.num_databases = 256,
 		.num_ports = 10,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6185,
 	}
 };
 
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index f75164d..e64cbee 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -24,24 +24,28 @@ static const struct mv88e6xxx_info mv88e6171_table[] = {
 		.name = "Marvell 88E6171",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6175,
 		.family = MV88E6XXX_FAMILY_6351,
 		.name = "Marvell 88E6175",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6350,
 		.family = MV88E6XXX_FAMILY_6351,
 		.name = "Marvell 88E6350",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6351,
 		.family = MV88E6XXX_FAMILY_6351,
 		.name = "Marvell 88E6351",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6351,
 	}
 };
 
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index c622a1d..c61f0f4 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -29,36 +29,42 @@ static const struct mv88e6xxx_info mv88e6352_table[] = {
 		.name = "Marvell 88E6320",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6320,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6321,
 		.family = MV88E6XXX_FAMILY_6320,
 		.name = "Marvell 88E6321",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6320,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6172,
 		.family = MV88E6XXX_FAMILY_6352,
 		.name = "Marvell 88E6172",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6176,
 		.family = MV88E6XXX_FAMILY_6352,
 		.name = "Marvell 88E6176",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6240,
 		.family = MV88E6XXX_FAMILY_6352,
 		.name = "Marvell 88E6240",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 	}, {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6352,
 		.family = MV88E6XXX_FAMILY_6352,
 		.name = "Marvell 88E6352",
 		.num_databases = 4096,
 		.num_ports = 7,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 	}
 };
 
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 4f455d2..c67b72a 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -350,12 +350,27 @@ enum mv88e6xxx_family {
 	MV88E6XXX_FAMILY_6352,	/* 6172 6176 6240 6352 */
 };
 
+#define MV88E6XXX_FLAGS_FAMILY_6095	0
+
+#define MV88E6XXX_FLAGS_FAMILY_6097	0
+
+#define MV88E6XXX_FLAGS_FAMILY_6165	0
+
+#define MV88E6XXX_FLAGS_FAMILY_6185	0
+
+#define MV88E6XXX_FLAGS_FAMILY_6320	0
+
+#define MV88E6XXX_FLAGS_FAMILY_6351	0
+
+#define MV88E6XXX_FLAGS_FAMILY_6352	0
+
 struct mv88e6xxx_info {
 	enum mv88e6xxx_family family;
 	u16 prod_num;
 	const char *name;
 	unsigned int num_databases;
 	unsigned int num_ports;
+	unsigned long flags;
 };
 
 struct mv88e6xxx_atu_entry {
@@ -449,6 +464,12 @@ struct mv88e6xxx_hw_stat {
 	enum stat_type type;
 };
 
+static inline bool mv88e6xxx_has(struct mv88e6xxx_priv_state *ps,
+				 unsigned long flags)
+{
+	return (ps->info->flags & flags) == flags;
+}
+
 int mv88e6xxx_switch_reset(struct mv88e6xxx_priv_state *ps, bool ppu_active);
 const char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
 				int sw_addr, void **priv,
-- 
2.8.2

^ permalink raw reply related

* [PATCH net-next 00/21] net: dsa: mv88e6xxx: turn into monolithic driver
From: Vivien Didelot @ 2016-05-06 21:57 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
	Andrew Lunn, Vivien Didelot

This patchset merges all mv88e6* drivers supported by the shared
mv88e6xxx code into a single mv88e6xxx DSA switch driver.

Some flags are added to describe the capabilities of a switch model,
such as the presence of a PPU, EEPROM, some old or new registers, etc.

First these flags are used to conditionally support the same set of
functions in every driver, then specific driver files are removed in
favor of the common mv88e6xxx driver.

Only the merge of driver specific setup code assumes a few differences.
If these differences such as frames priorities are really needed for
some models, they can easily be brought back in a future patch.

Some inconsistencies might show up, such as the need for
MV88E6XXX_FLAG_PPU and MV88E6XXX_FLAG_PPU_ACTIVE flags. But this
patchset does not aim to fix them yet. A future patch can do that if
they end up being unwanted.

The patchset has been tested on interconnected 88E6352 and 88E6185.

Changes RFC -> v1:
  - introduce flags in a separate patch
  - do not refactor anything yet
  - do not add new functions prefixed with _
  - drop packet discarding and mentioned tested platforms
  - factorize family flags
  - update text for NET_DSA_MV88E6XXX Kconfig entry

Vivien Didelot (21):
  net: dsa: mv88e6xxx: add flags to info
  net: dsa: mv88e6xxx: factorize PHY access with PPU
  net: dsa: mv88e6xxx: factorize PHY indirect access
  net: dsa: mv88e6xxx: factorize EEPROM access
  net: dsa: mv88e6xxx: factorize temperature access
  net: dsa: mv88e6xxx: factorize MAC address setting
  net: dsa: mv88e6xxx: factorize EEE access
  net: dsa: mv88e6xxx: factorize registers access
  net: dsa: mv88e6xxx: factorize bridge support
  net: dsa: mv88e6xxx: factorize VTU access
  net: dsa: mv88e6xxx: factorize ATU access
  net: dsa: mv88e6xxx: factorize switch reset
  net: dsa: mv88e6xxx: factorize global setup
  net: dsa: mv88e6xxx: factorize GLOBAL_CONTROL setup
  net: dsa: mv88e6xxx: factorize VLAN Ethertype
  net: dsa: mv88e6xxx: factorize GLOBAL_MONITOR_CONTROL setup
  net: dsa: mv88e6xxx: factorize GLOBAL_CONTROL_2 setup
  net: dsa: mv88e6xxx: factorize frames priorities
  net: dsa: mv88e6xxx: factorize switch setup
  net: dsa: mv88e6xxx: factorize tag protocol
  net: dsa: mv88e6xxx: factorize the switch driver

 arch/arm/configs/multi_v5_defconfig |   5 +-
 arch/arm/configs/mvebu_v7_defconfig |   2 +-
 arch/arm/configs/orion5x_defconfig  |   3 +-
 arch/tile/configs/tilegx_defconfig  |   3 +-
 arch/tile/configs/tilepro_defconfig |   3 +-
 drivers/net/dsa/Kconfig             |  45 +-
 drivers/net/dsa/Makefile            |  15 +-
 drivers/net/dsa/mv88e6123.c         | 130 -----
 drivers/net/dsa/mv88e6131.c         | 204 --------
 drivers/net/dsa/mv88e6171.c         | 151 ------
 drivers/net/dsa/mv88e6352.c         | 377 --------------
 drivers/net/dsa/mv88e6xxx.c         | 984 +++++++++++++++++++++++++++---------
 drivers/net/dsa/mv88e6xxx.h         | 249 ++++++---
 13 files changed, 908 insertions(+), 1263 deletions(-)
 delete mode 100644 drivers/net/dsa/mv88e6123.c
 delete mode 100644 drivers/net/dsa/mv88e6131.c
 delete mode 100644 drivers/net/dsa/mv88e6171.c
 delete mode 100644 drivers/net/dsa/mv88e6352.c

-- 
2.8.2

^ permalink raw reply

* Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions
From: John Stultz @ 2016-05-06 20:57 UTC (permalink / raw)
  To: Dean Jenkins
  Cc: David B. Robins, lkml, Mark Craske, David S. Miller, YongQin Liu,
	Guodong Xu, linux-usb, netdev, Ivan Vecera
In-Reply-To: <57291539.6080405@mentor.com>

On Tue, May 3, 2016 at 2:16 PM, Dean Jenkins <Dean_Jenkins@mentor.com> wrote:
>
> [  239.027993] asix 1-1.1:1.0 eth0: asix_rx_fixup() Data Header
> synchronisation was lost, remaining 988
>
> This error message consistently shows the remaining value to be 988, at
> least for the 3 examples provided by John. This does not suggest a random
> failure unless there are other examples of a non 988 remaining value error
> message. 988 is well within a Ethernet frame length so seems to be valid.
>
> I think a good step would be to add some debug to print the rx->remaining
> value at entry to asix_rx_fixup_internal(). This would generate a lot of
> debug but a pattern of the values might emerge.


So I've been trying to add some print messages here to better
understand whats going on.

Again, I'm a bit new to this code, so forgive me for my lack of
understanding things. Since the remaining value seems to be key, I
tried to look around and figure out where it was being set. It seems
like its only set in this function, is that right?  So this made me
guess something might be happening in a previous iteration that was
causing this to trigger.

I added some debug prints to every time we set the remaining value, or
modify it, as well as to print the value if we enter the fixup
function with a non-zero remaining value.

When we set the remaining value, usually its to 1514, when the skblen is 1518.

However, right before we catch the problem, I see this:


[   84.844337] JDB set remaining to 1514 (skblen: 1518)
[   84.844379] JDB set remaining to 1514 (skblen: 1518)
[   84.844429] JDB set remaining to 1514 (skblen: 1518)
[   84.844458] JDB set remaining to 1514 (skblen: 1518)
[   84.844483] JDB set remaining to 1514 (skblen: 1518)
[   84.844507] JDB set remaining to 1514 (skblen: 1518)
[   84.844559] JDB set remaining to 1514 (skblen: 2048)
[   84.844583] JDB set remaining to 1514 (skblen: 2048)
[   84.844595] JDB: 1514 > 2048 - 1522
[   84.844606] JDB: dropping remaining by 526
[   84.844624] asix_rx_fixup_internal()  remaining: 988,  skb->len: 2048
[   84.844672] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[   84.844945] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xdd5f8f9b, offset 4
[   84.845217] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x3ce1ad3c, offset 4
[   84.845451] JDB set remaining to 1514 (skblen: 1518)
[   84.845485] JDB set remaining to 1514 (skblen: 1518)
[   84.845511] JDB set remaining to 1514 (skblen: 1518)
[   84.851003] JDB set remaining to 1514 (skblen: 1518)


So when the issue happens, it seems to be due to an larger then usual
skb (2048). The first time through the wile loop we set the remaining
to 1514, but offset is fairly small, so we set copy_length to 1514,
and clear remaining. The offset is bumped by a little more then the
copy length and we loop again.  Then the second time through we set
remaining to 1514, but since offset is bigger now, the if
(rx->remaining > skb->len - offset)  case is true..

This is where it feels a little strange..

We calculate the copy_length as the difference between the offset and
the skb->len (so how much is left in the skb, which is 526), then
decrement remaining by that amount.  Not really sure what remaining
(now 988) is supposed to represent here. We copy the 526 bytes, and
then exit the loop.

Now the next time we are called, we enter and we have a remaining
value still of 988, which triggers the header synchronization error
path.

Now, I'm not sure if the remainder handling logic is sane, or if the
skb->len being 2048 is problematic, or what.  The skb->lens can vary
in sizes, usually 1518 during high throughput, but I've seen 1588,
1640, and other larger numbers that don't trigger the same problem.

ie:
[  106.946473] JDB set remaining to 1514 (skblen: 1518)
[  106.946525] JDB set remaining to 1514 (skblen: 1640)
[  106.946546] JDB set remaining to 118 (skblen: 1640)
[  106.946586] JDB set remaining to 1514 (skblen: 1518)


So yea.. maybe that will help clue things in a bit? I'm still a bit lost. :)

thanks
-john

^ permalink raw reply

* Re: [RFC PATCH net-next 19/20] net: dsa: mv88e6xxx: factorize tag protocol
From: Vivien Didelot @ 2016-05-06 20:56 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20160506010949.GM7972@lunn.ch>

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> On Thu, May 05, 2016 at 06:41:03PM -0400, Vivien Didelot wrote:
>> 6131 is the only driver to set the tag protocol to DSA_TAG_PROTO_DSA.
>> Since it works fine with DSA_TAG_PROTO_EDSA, change its value, like all
>> other mv88e6xxx drivers.
>
> You might as well remove net/dsa/tag_dsa.c as well, since it is now
> unused. Please do that as a separate patch.

Why not, but isn't it a bit out of the scope of this patchset?

Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH v2 next-next 02/12] net: define gso types for IPx over IPv4 and IPv6
From: Alexander Duyck @ 2016-05-06 20:54 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <CALx6S374bWLRk2T6UhNa30e-rRRyNe6cemuqRHbphkAfo6991w@mail.gmail.com>

On Fri, May 6, 2016 at 1:43 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Fri, May 6, 2016 at 1:34 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Fri, May 6, 2016 at 1:04 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> This patch defines two new GDO definitions SKB_GSO_IPXIP4 and
>>> SKB_GSO_IPXIP6 along with corresponding NETIF_F_GSO_IPXIP4 and
>>> NETIF_F_GSO_IPXIP6. These are used to described IP in IP
>>> tunnel and what the outer protocol is. The inner protocol
>>> can be deduced from other GSO types (e.g. SKB_GSO_TCPV4 and
>>> SKB_GSO_TCPV6). The GSO types of SKB_GSO_IPIP and SKB_GSO_SIT
>>> are removed (these are both instances of SKB_GSO_IPXIP4).
>>> SKB_GSO_IPXIP6 will be used when support for GSO with IP
>>> encapsulation over IPv6 is added.
>>>
>>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>>> ---
>>>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  |  5 ++---
>>>  drivers/net/ethernet/broadcom/bnxt/bnxt.c         |  4 ++--
>>>  drivers/net/ethernet/intel/i40e/i40e_main.c       |  3 +--
>>>  drivers/net/ethernet/intel/i40e/i40e_txrx.c       |  3 +--
>>>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c     |  3 +--
>>>  drivers/net/ethernet/intel/i40evf/i40evf_main.c   |  3 +--
>>>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |  3 +--
>>>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  3 +--
>>>  include/linux/netdev_features.h                   | 12 ++++++------
>>>  include/linux/netdevice.h                         |  4 ++--
>>>  include/linux/skbuff.h                            |  4 ++--
>>>  net/core/ethtool.c                                |  4 ++--
>>>  net/ipv4/af_inet.c                                |  2 +-
>>>  net/ipv4/ipip.c                                   |  2 +-
>>>  net/ipv6/ip6_offload.c                            |  4 ++--
>>>  net/ipv6/sit.c                                    |  4 ++--
>>>  net/netfilter/ipvs/ip_vs_xmit.c                   | 11 ++---------
>>>  17 files changed, 30 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
>>> index d465bd7..0a5b770 100644
>>> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
>>> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
>>> @@ -13259,12 +13259,11 @@ static int bnx2x_init_dev(struct bnx2x *bp, struct pci_dev *pdev,
>>>                 NETIF_F_RXHASH | NETIF_F_HW_VLAN_CTAG_TX;
>>>         if (!chip_is_e1x) {
>>>                 dev->hw_features |= NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL |
>>> -                                   NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT;
>>> +                                   NETIF_F_GSO_IPXIP4;
>>>                 dev->hw_enc_features =
>>>                         NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG |
>>>                         NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 |
>>> -                       NETIF_F_GSO_IPIP |
>>> -                       NETIF_F_GSO_SIT |
>>> +                       NETIF_F_GSO_IPXIP4 |
>>>                         NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL;
>>>         }
>>>
>>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>>> index fd85b6d..e449228 100644
>>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>>> @@ -6218,7 +6218,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>         dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG |
>>>                            NETIF_F_TSO | NETIF_F_TSO6 |
>>>                            NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
>>> -                          NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
>>> +                          NETIF_F_GSO_IPXIP4 |
>>>                            NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
>>>                            NETIF_F_GSO_PARTIAL | NETIF_F_RXHASH |
>>>                            NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_GRO;
>>> @@ -6228,7 +6228,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>                         NETIF_F_TSO | NETIF_F_TSO6 |
>>>                         NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
>>>                         NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
>>> -                       NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
>>> +                       NETIF_F_GSO_IPXIP4;
>>>                         NETIF_F_GSO_PARTIAL;
>>>         dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM |
>>>                                     NETIF_F_GSO_GRE_CSUM;
>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>>> index f6da6b7..c2a4c10 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>>> @@ -9131,8 +9131,7 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
>>>                                    NETIF_F_TSO6                 |
>>>                                    NETIF_F_GSO_GRE              |
>>>                                    NETIF_F_GSO_GRE_CSUM         |
>>> -                                  NETIF_F_GSO_IPIP             |
>>> -                                  NETIF_F_GSO_SIT              |
>>> +                                  NETIF_F_GSO_IPXIP4           |
>>>                                    NETIF_F_GSO_UDP_TUNNEL       |
>>>                                    NETIF_F_GSO_UDP_TUNNEL_CSUM  |
>>>                                    NETIF_F_GSO_PARTIAL          |
>>
>> So for all the Intel drivers they support IPv6 and IPv4 outer header
>> tunnel types.  As such you can add SKB_GSO_IPXIP6 as well and you
>> should be able to see offloads without any issues.  This applies for
>> all the Intel drivers below and igb which should have a patch
>> available sometime soon as it is still in Jeff Kirsher's tree.
>>
> Can we do this in a follow up patch?

The Intel driver piece yes, but the problem is there was also the bit
at the end where you deleted a comment about needing an IPv6 type and
didn't add an IPv6 type.  That is one piece I would be a bit more of a
stickler on as that comment is there for a reason and you are removing
it without resolving the issue it is meant to flag.

>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>>> index 2765d7e..4a3d60a 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>>> @@ -2302,8 +2302,7 @@ static int i40e_tso(struct sk_buff *skb, u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
>>>
>>>         if (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE |
>>>                                          SKB_GSO_GRE_CSUM |
>>> -                                        SKB_GSO_IPIP |
>>> -                                        SKB_GSO_SIT |
>>> +                                        SKB_GSO_IPXIP4 |
>>>                                          SKB_GSO_UDP_TUNNEL |
>>>                                          SKB_GSO_UDP_TUNNEL_CSUM)) {
>>>                 if (!(skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) &&
>>> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
>>> index ede8dfc..aaccb7e 100644
>>> --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
>>> +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
>>> @@ -1567,8 +1567,7 @@ static int i40e_tso(struct sk_buff *skb, u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
>>>
>>>         if (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE |
>>>                                          SKB_GSO_GRE_CSUM |
>>> -                                        SKB_GSO_IPIP |
>>> -                                        SKB_GSO_SIT |
>>> +                                        SKB_GSO_IPXIP4 |
>>>                                          SKB_GSO_UDP_TUNNEL |
>>>                                          SKB_GSO_UDP_TUNNEL_CSUM)) {
>>>                 if (!(skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) &&
>>> diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
>>> index 9f0bd7a..bfd0962 100644
>>> --- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
>>> +++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
>>> @@ -2241,8 +2241,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
>>>                                    NETIF_F_TSO6                 |
>>>                                    NETIF_F_GSO_GRE              |
>>>                                    NETIF_F_GSO_GRE_CSUM         |
>>> -                                  NETIF_F_GSO_IPIP             |
>>> -                                  NETIF_F_GSO_SIT              |
>>> +                                  NETIF_F_GSO_IPXIP4           |
>>>                                    NETIF_F_GSO_UDP_TUNNEL       |
>>>                                    NETIF_F_GSO_UDP_TUNNEL_CSUM  |
>>>                                    NETIF_F_GSO_PARTIAL          |
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index d08fbcf..28e3b5a 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -9482,8 +9482,7 @@ skip_sriov:
>>>
>>>  #define IXGBE_GSO_PARTIAL_FEATURES (NETIF_F_GSO_GRE | \
>>>                                     NETIF_F_GSO_GRE_CSUM | \
>>> -                                   NETIF_F_GSO_IPIP | \
>>> -                                   NETIF_F_GSO_SIT | \
>>> +                                   NETIF_F_GSO_IPXIP4 | \
>>>                                     NETIF_F_GSO_UDP_TUNNEL | \
>>>                                     NETIF_F_GSO_UDP_TUNNEL_CSUM)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> index 5e348b1..d86e511 100644
>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>>> @@ -4062,8 +4062,7 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>
>>>  #define IXGBEVF_GSO_PARTIAL_FEATURES (NETIF_F_GSO_GRE | \
>>>                                       NETIF_F_GSO_GRE_CSUM | \
>>> -                                     NETIF_F_GSO_IPIP | \
>>> -                                     NETIF_F_GSO_SIT | \
>>> +                                     NETIF_F_GSO_IPXIP4 | \
>>>                                       NETIF_F_GSO_UDP_TUNNEL | \
>>>                                       NETIF_F_GSO_UDP_TUNNEL_CSUM)
>>>
>>
>> So with GSO partial all of the igb/ixgbe parts should be able to
>> support any tunnel type you can throw at it.
>>
>>> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>>> index bc87362..aa7b240 100644
>>> --- a/include/linux/netdev_features.h
>>> +++ b/include/linux/netdev_features.h
>>> @@ -44,8 +44,8 @@ enum {
>>>         NETIF_F_FSO_BIT,                /* ... FCoE segmentation */
>>>         NETIF_F_GSO_GRE_BIT,            /* ... GRE with TSO */
>>>         NETIF_F_GSO_GRE_CSUM_BIT,       /* ... GRE with csum with TSO */
>>> -       NETIF_F_GSO_IPIP_BIT,           /* ... IPIP tunnel with TSO */
>>> -       NETIF_F_GSO_SIT_BIT,            /* ... SIT tunnel with TSO */
>>> +       NETIF_F_GSO_IPXIP4_BIT,         /* ... IP4 or IP6 over IP4 with TSO */
>>> +       NETIF_F_GSO_IPXIP6_BIT,         /* ... IP4 or IP6 over IP6 with TSO */
>>>         NETIF_F_GSO_UDP_TUNNEL_BIT,     /* ... UDP TUNNEL with TSO */
>>>         NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT,/* ... UDP TUNNEL with TSO & CSUM */
>>>         NETIF_F_GSO_PARTIAL_BIT,        /* ... Only segment inner-most L4
>>> @@ -121,8 +121,8 @@ enum {
>>>  #define NETIF_F_RXALL          __NETIF_F(RXALL)
>>>  #define NETIF_F_GSO_GRE                __NETIF_F(GSO_GRE)
>>>  #define NETIF_F_GSO_GRE_CSUM   __NETIF_F(GSO_GRE_CSUM)
>>> -#define NETIF_F_GSO_IPIP       __NETIF_F(GSO_IPIP)
>>> -#define NETIF_F_GSO_SIT                __NETIF_F(GSO_SIT)
>>> +#define NETIF_F_GSO_IPXIP4     __NETIF_F(GSO_IPXIP4)
>>> +#define NETIF_F_GSO_IPXIP6     __NETIF_F(GSO_IPXIP6)
>>>  #define NETIF_F_GSO_UDP_TUNNEL __NETIF_F(GSO_UDP_TUNNEL)
>>>  #define NETIF_F_GSO_UDP_TUNNEL_CSUM __NETIF_F(GSO_UDP_TUNNEL_CSUM)
>>>  #define NETIF_F_TSO_MANGLEID   __NETIF_F(TSO_MANGLEID)
>>> @@ -200,8 +200,8 @@ enum {
>>>
>>>  #define NETIF_F_GSO_ENCAP_ALL  (NETIF_F_GSO_GRE |                      \
>>>                                  NETIF_F_GSO_GRE_CSUM |                 \
>>> -                                NETIF_F_GSO_IPIP |                     \
>>> -                                NETIF_F_GSO_SIT |                      \
>>> +                                NETIF_F_GSO_IPXIP4 |                   \
>>> +                                NETIF_F_GSO_IPXIP6 |                   \
>>>                                  NETIF_F_GSO_UDP_TUNNEL |               \
>>>                                  NETIF_F_GSO_UDP_TUNNEL_CSUM)
>>>
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index 63580e6..8b1eedc 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -4005,8 +4005,8 @@ static inline bool net_gso_ok(netdev_features_t features, int gso_type)
>>>         BUILD_BUG_ON(SKB_GSO_FCOE    != (NETIF_F_FSO >> NETIF_F_GSO_SHIFT));
>>>         BUILD_BUG_ON(SKB_GSO_GRE     != (NETIF_F_GSO_GRE >> NETIF_F_GSO_SHIFT));
>>>         BUILD_BUG_ON(SKB_GSO_GRE_CSUM != (NETIF_F_GSO_GRE_CSUM >> NETIF_F_GSO_SHIFT));
>>> -       BUILD_BUG_ON(SKB_GSO_IPIP    != (NETIF_F_GSO_IPIP >> NETIF_F_GSO_SHIFT));
>>> -       BUILD_BUG_ON(SKB_GSO_SIT     != (NETIF_F_GSO_SIT >> NETIF_F_GSO_SHIFT));
>>> +       BUILD_BUG_ON(SKB_GSO_IPXIP4  != (NETIF_F_GSO_IPXIP4 >> NETIF_F_GSO_SHIFT));
>>> +       BUILD_BUG_ON(SKB_GSO_IPXIP6  != (NETIF_F_GSO_IPXIP6 >> NETIF_F_GSO_SHIFT));
>>>         BUILD_BUG_ON(SKB_GSO_UDP_TUNNEL != (NETIF_F_GSO_UDP_TUNNEL >> NETIF_F_GSO_SHIFT));
>>>         BUILD_BUG_ON(SKB_GSO_UDP_TUNNEL_CSUM != (NETIF_F_GSO_UDP_TUNNEL_CSUM >> NETIF_F_GSO_SHIFT));
>>>         BUILD_BUG_ON(SKB_GSO_PARTIAL != (NETIF_F_GSO_PARTIAL >> NETIF_F_GSO_SHIFT));
>>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>>> index c413c58..65968a9 100644
>>> --- a/include/linux/skbuff.h
>>> +++ b/include/linux/skbuff.h
>>> @@ -471,9 +471,9 @@ enum {
>>>
>>>         SKB_GSO_GRE_CSUM = 1 << 8,
>>>
>>> -       SKB_GSO_IPIP = 1 << 9,
>>> +       SKB_GSO_IPXIP4 = 1 << 9,
>>>
>>> -       SKB_GSO_SIT = 1 << 10,
>>> +       SKB_GSO_IPXIP6 = 1 << 10,
>>>
>>>         SKB_GSO_UDP_TUNNEL = 1 << 11,
>>>
>>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>>> index bdb4013..f403481 100644
>>> --- a/net/core/ethtool.c
>>> +++ b/net/core/ethtool.c
>>> @@ -84,8 +84,8 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
>>>         [NETIF_F_FSO_BIT] =              "tx-fcoe-segmentation",
>>>         [NETIF_F_GSO_GRE_BIT] =          "tx-gre-segmentation",
>>>         [NETIF_F_GSO_GRE_CSUM_BIT] =     "tx-gre-csum-segmentation",
>>> -       [NETIF_F_GSO_IPIP_BIT] =         "tx-ipip-segmentation",
>>> -       [NETIF_F_GSO_SIT_BIT] =          "tx-sit-segmentation",
>>> +       [NETIF_F_GSO_IPXIP4_BIT] =       "tx-ipxip4-segmentation",
>>> +       [NETIF_F_GSO_IPXIP6_BIT] =       "tx-ipxip6-segmentation",
>>>         [NETIF_F_GSO_UDP_TUNNEL_BIT] =   "tx-udp_tnl-segmentation",
>>>         [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
>>>         [NETIF_F_GSO_PARTIAL_BIT] =      "tx-gso-partial",
>>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>>> index 7f08d45..25040b1 100644
>>> --- a/net/ipv4/af_inet.c
>>> +++ b/net/ipv4/af_inet.c
>>> @@ -1483,7 +1483,7 @@ out_unlock:
>>>  static int ipip_gro_complete(struct sk_buff *skb, int nhoff)
>>>  {
>>>         skb->encapsulation = 1;
>>> -       skb_shinfo(skb)->gso_type |= SKB_GSO_IPIP;
>>> +       skb_shinfo(skb)->gso_type |= SKB_GSO_IPXIP4;
>>>         return inet_gro_complete(skb, nhoff);
>>>  }
>>>
>>> diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
>>> index 9282748..9783701 100644
>>> --- a/net/ipv4/ipip.c
>>> +++ b/net/ipv4/ipip.c
>>> @@ -219,7 +219,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
>>>         if (unlikely(skb->protocol != htons(ETH_P_IP)))
>>>                 goto tx_error;
>>>
>>> -       if (iptunnel_handle_offloads(skb, SKB_GSO_IPIP))
>>> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4))
>>>                 goto tx_error;
>>>
>>>         skb_set_inner_ipproto(skb, IPPROTO_IPIP);
>>> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
>>> index 9ad743b..787e55f 100644
>>> --- a/net/ipv6/ip6_offload.c
>>> +++ b/net/ipv6/ip6_offload.c
>>> @@ -86,7 +86,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>>>         proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
>>>
>>>         if (skb->encapsulation &&
>>> -           skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
>>> +           skb_shinfo(skb)->gso_type & (SKB_GSO_IPXIP4 | SKB_GSO_IPXIP6))
>>>                 udpfrag = proto == IPPROTO_UDP && encap;
>>>         else
>>>                 udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
>>> @@ -294,7 +294,7 @@ out_unlock:
>>>  static int sit_gro_complete(struct sk_buff *skb, int nhoff)
>>>  {
>>>         skb->encapsulation = 1;
>>> -       skb_shinfo(skb)->gso_type |= SKB_GSO_SIT;
>>> +       skb_shinfo(skb)->gso_type |= SKB_GSO_IPXIP4;
>>>         return ipv6_gro_complete(skb, nhoff);
>>>  }
>>>
>>> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
>>> index a13d8c1..0a5a255 100644
>>> --- a/net/ipv6/sit.c
>>> +++ b/net/ipv6/sit.c
>>> @@ -913,7 +913,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
>>>                 goto tx_error;
>>>         }
>>>
>>> -       if (iptunnel_handle_offloads(skb, SKB_GSO_SIT)) {
>>> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4)) {
>>>                 ip_rt_put(rt);
>>>                 goto tx_error;
>>>         }
>>> @@ -1000,7 +1000,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
>>>         struct ip_tunnel *tunnel = netdev_priv(dev);
>>>         const struct iphdr  *tiph = &tunnel->parms.iph;
>>>
>>> -       if (iptunnel_handle_offloads(skb, SKB_GSO_IPIP))
>>> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4))
>>>                 goto tx_error;
>>>
>>>         skb_set_inner_ipproto(skb, IPPROTO_IPIP);
>>> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
>>> index 6d19d2e..f646ef6 100644
>>> --- a/net/netfilter/ipvs/ip_vs_xmit.c
>>> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
>>> @@ -932,16 +932,9 @@ error:
>>>
>>>  static inline int __tun_gso_type_mask(int encaps_af, int orig_af)
>>>  {
>>> -       if (encaps_af == AF_INET) {
>>> -               if (orig_af == AF_INET)
>>> -                       return SKB_GSO_IPIP;
>>> +       if (encaps_af == AF_INET)
>>> +               return SKB_GSO_IPXIP4;
>>>
>>> -               return SKB_GSO_SIT;
>>> -       }
>>> -
>>> -       /* GSO: we need to provide proper SKB_GSO_ value for IPv6:
>>> -        * SKB_GSO_SIT/IPV6
>>> -        */
>>
>> I wouldn't delete this comment until you actually have resolved the
>> issue of providing a proper GSO value for IPv6.
>>
>>>         return 0;
>>>  }
>>>
>>> --
>>> 2.8.0.rc2
>>>

^ permalink raw reply

* Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions
From: David B. Robins @ 2016-05-06 20:45 UTC (permalink / raw)
  To: Dean Jenkins
  Cc: John Stultz, lkml, Mark Craske, David S. Miller, YongQin Liu,
	Guodong Xu, linux-usb, netdev, Ivan Vecera
In-Reply-To: <5729AB90.9010704@mentor.com>

On 2016-05-04 03:58, Dean Jenkins wrote:
> On 04/05/16 01:28, David B. Robins wrote:
>> 
>>> 
>>> Here is the code snippet from the patch with my annotations between #
>>> #, I will try to explain my intentions. Feel free to point out any
>>> flaws:
>>> 
>>>     if (rx->remaining && (rx->remaining + sizeof(u32) <= skb->len)) {
>>>         # Only runs when rx->remaining !=0 and the end of the 
>>> Ethernet
>>> frame + next 32-bit header word is within the URB buffer. #
>>>         # Therefore, this code does not run when the end of an
>>> Ethernet frame has been reached in the previous URB #
>>>         # or when the end of the Ethernet frame + next 32-bit header
>>> word will be in a later URB buffer #
>> 
>> It may well be. I don't have the setup with me now, but I can try 
>> tomorrow to reproduce an environment where I can add some more 
>> detailed logging.
>> 
>> Since the URB length has to be >= than the remaining data plus a u32, 
>> the devices that John Stultz and I are using (AX88772B in my case) may 
>> be adding some additional data/padding after an Ethernet frame, 
>> expecting it to be discarded, and running into this check and its 
>> consequences. This may mean the device is badly behaved, if it is 
>> specified not to send anything extra; in any case, a well-intentioned 
>> error correction has gone badly, but I better understand the intent 
>> now. I am curious to know how often the device you are using benefits 
>> from this block of code.
> 
> The issue is that the driver should be robust to cope with missing
> URBs. Whilst testing with D-Link DUB-E100 C1 AX88772 USB to Ethernet
> adaptor in our ARM embedded system which runs in hostile environments,
> it was noticed that URBs could be lost (probably due to a bug
> elsewhere or low memory issue). Without this patch, a missing URB
> causes bad Ethernet frames to be passed up to the IP stack because
> rx->remaining spans multiple URBs.
> 
> In the good case of an Ethernet frame spanning 2 URBs, the 1st URB is
> processed and copies the 1st part of the Ethernet frame into the
> netdev buffer, for the 2nd URB the remaining part of the Ethernet
> frame is copied into the same netdev buffer to complete the Ethernet
> frame. The netdev buffer is then sent up to the IP stack.
> 
> In the case of a missing URB, a bad Ethernet frame is created as 
> follows:
> The 1st URB is processed and copies the 1st part of the Ethernet frame
> into the netdev buffer, the 2nd URB is lost (somehow),  the 3rd URB is
> processed and blindly copies what it thinks is the remaining part of
> the Ethernet frame in the same netdev buffer which corrupts the
> Ethernet frame. The netdev buffer is then sent up to the IP stack. The
> 3rd URB and subsequent URBs are processed but synchronisation has been
> lost so can misread data as a 32-bit header word. It is likely that
> some good Ethernet frames get discarded whilst trying to
> resynchronise.
> 
> A recovery strategy for regaining lock with the 32-bit header word is
> necessary otherwise the driver will have difficulty in recovering from
> a lost URB.
> 
> In the "olden days", the 32-bit header word was always at the start of
> the URB buffer so previous URBs did not influence the current URB. So
> no recovery strategy was needed at that time. But now we have to
> remember what happened in the previous URB and a lost URB can cause a
> discontinuity in the data stream because the data is not always
> aligned to the start of the URB buffer.
> 
> I agree that your environment may never suffer from lost URBs so
> removal of the patch would work OK.
> 
> I will try to find some time to setup a test environment.

I got the chance to add some more logging (and add back the header 
synchronization code) to a system which is receiving video data on an 
8-port POE device using AX88772Bs - it replaces the "Data header 
synchronization was lost" log and logs a few more local items ("u32" = 
rx->header):

[3105478.073908] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3105478.073918] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() Bad Header 
Length 0xf05b7c5f, offset 4
[3105478.074022] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() skb->len = 
2048
[3105478.074032] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() Bad Header 
Length 0xffaae2a5, offset 4
[3105478.074146] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() skb->len = 
1518
[3105478.074280] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() skb->len = 
2048
[3105478.074291] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() Bad Header 
Length 0xe1478400, offset 4
[3105478.074399] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() skb->len = 916
[3105478.074409] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() Bad Header 
Length 0x9fa84eb5, offset 4
[3105478.074657] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() skb->len = 
1518

Grepping for just the SYNC_LOSS messages shows a similar pattern to that 
seen elsewhere in the thread:

[3104761.056033] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.058035] asix 1-1.4.1.3:1.0 poe2: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.076795] asix 1-1.4.2.3:1.0 poe4: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.107904] asix 1-1.4.2.3:1.0 poe4: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.114649] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.121152] asix 1-1.4.1.1:1.0 poe1: asix_rx_fixup() SYNC_LOSS 
remain 988 len 1518 offset 992 u32 0xecf95000
[3104761.147784] asix 1-1.4.1.3:1.0 poe2: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.160535] asix 1-1.4.1.1:1.0 poe1: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.168024] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.168403] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.170654] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.180418] asix 1-1.4.1.1:1.0 poe1: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.221783] asix 1-1.4.1.1:1.0 poe1: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.248784] asix 1-1.4.2.2:1.0 poe7: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.252421] asix 1-1.4.1.2:1.0 poe5: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.262903] asix 1-1.4.2.4:1.0 poe8: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.266904] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.267281] asix 1-1.4.2.4:1.0 poe8: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.271665] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.273915] asix 1-1.4.2.4:1.0 poe8: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.280048] asix 1-1.4.2.4:1.0 poe8: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.287152] asix 1-1.4.2.3:1.0 poe4: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.292157] asix 1-1.4.2.3:1.0 poe4: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.295054] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.310915] asix 1-1.4.1.1:1.0 poe1: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2022 offset 992 u32 0xecf95000
[3104761.316918] asix 1-1.4.2.3:1.0 poe4: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.324935] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.326665] asix 1-1.4.2.3:1.0 poe4: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000
[3104761.336420] asix 1-1.4.1.4:1.0 poe6: asix_rx_fixup() SYNC_LOSS 
remain 988 len 2048 offset 992 u32 0xecf95000

This is an Ubuntu 14.04.2 LTS i686 (32-bit) system running kernel 
3.13.0-53-generic. I don't see any "low-speed" warnings, and lsusb -t 
shows each device (and their hubs) running at 480M.

ifconfig poe1:

poe1      Link encap:Ethernet  HWaddr 00:50:f9:ec:4f:d8
           inet addr:192.168.201.1  Bcast:192.168.201.255  
Mask:255.255.255.0
           inet6 addr: fe80::250:f9ff:feec:4fd8/64 Scope:Link
           UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
           RX packets:194441 errors:16649 dropped:15 overruns:0 frame:0
           TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
           collisions:0 txqueuelen:1000
           RX bytes:267310738 (267.3 MB)  TX bytes:10300282 (10.3 MB)

Without the "sanity test/synchronization" code:

poe1      Link encap:Ethernet  HWaddr 00:50:f9:ec:4f:d8
           inet addr:192.168.201.1  Bcast:192.168.201.255  
Mask:255.255.255.0
           inet6 addr: fe80::250:f9ff:feec:4fd8/64 Scope:Link
           UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
           RX packets:398910 errors:1 dropped:14 overruns:0 frame:0
           TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
           collisions:0 txqueuelen:1000
           RX bytes:513363997 (513.3 MB)  TX bytes:13154142 (13.1 MB)

> 
> Regards,
> Dean

David

^ permalink raw reply

* Re: [PATCH v2 next-next 02/12] net: define gso types for IPx over IPv4 and IPv6
From: Tom Herbert @ 2016-05-06 20:43 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <CAKgT0UdXWPqBN9SN45U80+fPOoukns38gZbR1B2abpCr1iDhcA@mail.gmail.com>

On Fri, May 6, 2016 at 1:34 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Fri, May 6, 2016 at 1:04 PM, Tom Herbert <tom@herbertland.com> wrote:
>> This patch defines two new GDO definitions SKB_GSO_IPXIP4 and
>> SKB_GSO_IPXIP6 along with corresponding NETIF_F_GSO_IPXIP4 and
>> NETIF_F_GSO_IPXIP6. These are used to described IP in IP
>> tunnel and what the outer protocol is. The inner protocol
>> can be deduced from other GSO types (e.g. SKB_GSO_TCPV4 and
>> SKB_GSO_TCPV6). The GSO types of SKB_GSO_IPIP and SKB_GSO_SIT
>> are removed (these are both instances of SKB_GSO_IPXIP4).
>> SKB_GSO_IPXIP6 will be used when support for GSO with IP
>> encapsulation over IPv6 is added.
>>
>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>> ---
>>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  |  5 ++---
>>  drivers/net/ethernet/broadcom/bnxt/bnxt.c         |  4 ++--
>>  drivers/net/ethernet/intel/i40e/i40e_main.c       |  3 +--
>>  drivers/net/ethernet/intel/i40e/i40e_txrx.c       |  3 +--
>>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c     |  3 +--
>>  drivers/net/ethernet/intel/i40evf/i40evf_main.c   |  3 +--
>>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |  3 +--
>>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  3 +--
>>  include/linux/netdev_features.h                   | 12 ++++++------
>>  include/linux/netdevice.h                         |  4 ++--
>>  include/linux/skbuff.h                            |  4 ++--
>>  net/core/ethtool.c                                |  4 ++--
>>  net/ipv4/af_inet.c                                |  2 +-
>>  net/ipv4/ipip.c                                   |  2 +-
>>  net/ipv6/ip6_offload.c                            |  4 ++--
>>  net/ipv6/sit.c                                    |  4 ++--
>>  net/netfilter/ipvs/ip_vs_xmit.c                   | 11 ++---------
>>  17 files changed, 30 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
>> index d465bd7..0a5b770 100644
>> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
>> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
>> @@ -13259,12 +13259,11 @@ static int bnx2x_init_dev(struct bnx2x *bp, struct pci_dev *pdev,
>>                 NETIF_F_RXHASH | NETIF_F_HW_VLAN_CTAG_TX;
>>         if (!chip_is_e1x) {
>>                 dev->hw_features |= NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL |
>> -                                   NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT;
>> +                                   NETIF_F_GSO_IPXIP4;
>>                 dev->hw_enc_features =
>>                         NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG |
>>                         NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 |
>> -                       NETIF_F_GSO_IPIP |
>> -                       NETIF_F_GSO_SIT |
>> +                       NETIF_F_GSO_IPXIP4 |
>>                         NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL;
>>         }
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>> index fd85b6d..e449228 100644
>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>> @@ -6218,7 +6218,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>         dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG |
>>                            NETIF_F_TSO | NETIF_F_TSO6 |
>>                            NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
>> -                          NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
>> +                          NETIF_F_GSO_IPXIP4 |
>>                            NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
>>                            NETIF_F_GSO_PARTIAL | NETIF_F_RXHASH |
>>                            NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_GRO;
>> @@ -6228,7 +6228,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>                         NETIF_F_TSO | NETIF_F_TSO6 |
>>                         NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
>>                         NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
>> -                       NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
>> +                       NETIF_F_GSO_IPXIP4;
>>                         NETIF_F_GSO_PARTIAL;
>>         dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM |
>>                                     NETIF_F_GSO_GRE_CSUM;
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index f6da6b7..c2a4c10 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -9131,8 +9131,7 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
>>                                    NETIF_F_TSO6                 |
>>                                    NETIF_F_GSO_GRE              |
>>                                    NETIF_F_GSO_GRE_CSUM         |
>> -                                  NETIF_F_GSO_IPIP             |
>> -                                  NETIF_F_GSO_SIT              |
>> +                                  NETIF_F_GSO_IPXIP4           |
>>                                    NETIF_F_GSO_UDP_TUNNEL       |
>>                                    NETIF_F_GSO_UDP_TUNNEL_CSUM  |
>>                                    NETIF_F_GSO_PARTIAL          |
>
> So for all the Intel drivers they support IPv6 and IPv4 outer header
> tunnel types.  As such you can add SKB_GSO_IPXIP6 as well and you
> should be able to see offloads without any issues.  This applies for
> all the Intel drivers below and igb which should have a patch
> available sometime soon as it is still in Jeff Kirsher's tree.
>
Can we do this in a follow up patch?

>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>> index 2765d7e..4a3d60a 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
>> @@ -2302,8 +2302,7 @@ static int i40e_tso(struct sk_buff *skb, u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
>>
>>         if (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE |
>>                                          SKB_GSO_GRE_CSUM |
>> -                                        SKB_GSO_IPIP |
>> -                                        SKB_GSO_SIT |
>> +                                        SKB_GSO_IPXIP4 |
>>                                          SKB_GSO_UDP_TUNNEL |
>>                                          SKB_GSO_UDP_TUNNEL_CSUM)) {
>>                 if (!(skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) &&
>> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
>> index ede8dfc..aaccb7e 100644
>> --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
>> +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
>> @@ -1567,8 +1567,7 @@ static int i40e_tso(struct sk_buff *skb, u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
>>
>>         if (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE |
>>                                          SKB_GSO_GRE_CSUM |
>> -                                        SKB_GSO_IPIP |
>> -                                        SKB_GSO_SIT |
>> +                                        SKB_GSO_IPXIP4 |
>>                                          SKB_GSO_UDP_TUNNEL |
>>                                          SKB_GSO_UDP_TUNNEL_CSUM)) {
>>                 if (!(skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) &&
>> diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
>> index 9f0bd7a..bfd0962 100644
>> --- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
>> +++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
>> @@ -2241,8 +2241,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
>>                                    NETIF_F_TSO6                 |
>>                                    NETIF_F_GSO_GRE              |
>>                                    NETIF_F_GSO_GRE_CSUM         |
>> -                                  NETIF_F_GSO_IPIP             |
>> -                                  NETIF_F_GSO_SIT              |
>> +                                  NETIF_F_GSO_IPXIP4           |
>>                                    NETIF_F_GSO_UDP_TUNNEL       |
>>                                    NETIF_F_GSO_UDP_TUNNEL_CSUM  |
>>                                    NETIF_F_GSO_PARTIAL          |
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index d08fbcf..28e3b5a 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -9482,8 +9482,7 @@ skip_sriov:
>>
>>  #define IXGBE_GSO_PARTIAL_FEATURES (NETIF_F_GSO_GRE | \
>>                                     NETIF_F_GSO_GRE_CSUM | \
>> -                                   NETIF_F_GSO_IPIP | \
>> -                                   NETIF_F_GSO_SIT | \
>> +                                   NETIF_F_GSO_IPXIP4 | \
>>                                     NETIF_F_GSO_UDP_TUNNEL | \
>>                                     NETIF_F_GSO_UDP_TUNNEL_CSUM)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> index 5e348b1..d86e511 100644
>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
>> @@ -4062,8 +4062,7 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>
>>  #define IXGBEVF_GSO_PARTIAL_FEATURES (NETIF_F_GSO_GRE | \
>>                                       NETIF_F_GSO_GRE_CSUM | \
>> -                                     NETIF_F_GSO_IPIP | \
>> -                                     NETIF_F_GSO_SIT | \
>> +                                     NETIF_F_GSO_IPXIP4 | \
>>                                       NETIF_F_GSO_UDP_TUNNEL | \
>>                                       NETIF_F_GSO_UDP_TUNNEL_CSUM)
>>
>
> So with GSO partial all of the igb/ixgbe parts should be able to
> support any tunnel type you can throw at it.
>
>> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>> index bc87362..aa7b240 100644
>> --- a/include/linux/netdev_features.h
>> +++ b/include/linux/netdev_features.h
>> @@ -44,8 +44,8 @@ enum {
>>         NETIF_F_FSO_BIT,                /* ... FCoE segmentation */
>>         NETIF_F_GSO_GRE_BIT,            /* ... GRE with TSO */
>>         NETIF_F_GSO_GRE_CSUM_BIT,       /* ... GRE with csum with TSO */
>> -       NETIF_F_GSO_IPIP_BIT,           /* ... IPIP tunnel with TSO */
>> -       NETIF_F_GSO_SIT_BIT,            /* ... SIT tunnel with TSO */
>> +       NETIF_F_GSO_IPXIP4_BIT,         /* ... IP4 or IP6 over IP4 with TSO */
>> +       NETIF_F_GSO_IPXIP6_BIT,         /* ... IP4 or IP6 over IP6 with TSO */
>>         NETIF_F_GSO_UDP_TUNNEL_BIT,     /* ... UDP TUNNEL with TSO */
>>         NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT,/* ... UDP TUNNEL with TSO & CSUM */
>>         NETIF_F_GSO_PARTIAL_BIT,        /* ... Only segment inner-most L4
>> @@ -121,8 +121,8 @@ enum {
>>  #define NETIF_F_RXALL          __NETIF_F(RXALL)
>>  #define NETIF_F_GSO_GRE                __NETIF_F(GSO_GRE)
>>  #define NETIF_F_GSO_GRE_CSUM   __NETIF_F(GSO_GRE_CSUM)
>> -#define NETIF_F_GSO_IPIP       __NETIF_F(GSO_IPIP)
>> -#define NETIF_F_GSO_SIT                __NETIF_F(GSO_SIT)
>> +#define NETIF_F_GSO_IPXIP4     __NETIF_F(GSO_IPXIP4)
>> +#define NETIF_F_GSO_IPXIP6     __NETIF_F(GSO_IPXIP6)
>>  #define NETIF_F_GSO_UDP_TUNNEL __NETIF_F(GSO_UDP_TUNNEL)
>>  #define NETIF_F_GSO_UDP_TUNNEL_CSUM __NETIF_F(GSO_UDP_TUNNEL_CSUM)
>>  #define NETIF_F_TSO_MANGLEID   __NETIF_F(TSO_MANGLEID)
>> @@ -200,8 +200,8 @@ enum {
>>
>>  #define NETIF_F_GSO_ENCAP_ALL  (NETIF_F_GSO_GRE |                      \
>>                                  NETIF_F_GSO_GRE_CSUM |                 \
>> -                                NETIF_F_GSO_IPIP |                     \
>> -                                NETIF_F_GSO_SIT |                      \
>> +                                NETIF_F_GSO_IPXIP4 |                   \
>> +                                NETIF_F_GSO_IPXIP6 |                   \
>>                                  NETIF_F_GSO_UDP_TUNNEL |               \
>>                                  NETIF_F_GSO_UDP_TUNNEL_CSUM)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 63580e6..8b1eedc 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -4005,8 +4005,8 @@ static inline bool net_gso_ok(netdev_features_t features, int gso_type)
>>         BUILD_BUG_ON(SKB_GSO_FCOE    != (NETIF_F_FSO >> NETIF_F_GSO_SHIFT));
>>         BUILD_BUG_ON(SKB_GSO_GRE     != (NETIF_F_GSO_GRE >> NETIF_F_GSO_SHIFT));
>>         BUILD_BUG_ON(SKB_GSO_GRE_CSUM != (NETIF_F_GSO_GRE_CSUM >> NETIF_F_GSO_SHIFT));
>> -       BUILD_BUG_ON(SKB_GSO_IPIP    != (NETIF_F_GSO_IPIP >> NETIF_F_GSO_SHIFT));
>> -       BUILD_BUG_ON(SKB_GSO_SIT     != (NETIF_F_GSO_SIT >> NETIF_F_GSO_SHIFT));
>> +       BUILD_BUG_ON(SKB_GSO_IPXIP4  != (NETIF_F_GSO_IPXIP4 >> NETIF_F_GSO_SHIFT));
>> +       BUILD_BUG_ON(SKB_GSO_IPXIP6  != (NETIF_F_GSO_IPXIP6 >> NETIF_F_GSO_SHIFT));
>>         BUILD_BUG_ON(SKB_GSO_UDP_TUNNEL != (NETIF_F_GSO_UDP_TUNNEL >> NETIF_F_GSO_SHIFT));
>>         BUILD_BUG_ON(SKB_GSO_UDP_TUNNEL_CSUM != (NETIF_F_GSO_UDP_TUNNEL_CSUM >> NETIF_F_GSO_SHIFT));
>>         BUILD_BUG_ON(SKB_GSO_PARTIAL != (NETIF_F_GSO_PARTIAL >> NETIF_F_GSO_SHIFT));
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index c413c58..65968a9 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -471,9 +471,9 @@ enum {
>>
>>         SKB_GSO_GRE_CSUM = 1 << 8,
>>
>> -       SKB_GSO_IPIP = 1 << 9,
>> +       SKB_GSO_IPXIP4 = 1 << 9,
>>
>> -       SKB_GSO_SIT = 1 << 10,
>> +       SKB_GSO_IPXIP6 = 1 << 10,
>>
>>         SKB_GSO_UDP_TUNNEL = 1 << 11,
>>
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index bdb4013..f403481 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -84,8 +84,8 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
>>         [NETIF_F_FSO_BIT] =              "tx-fcoe-segmentation",
>>         [NETIF_F_GSO_GRE_BIT] =          "tx-gre-segmentation",
>>         [NETIF_F_GSO_GRE_CSUM_BIT] =     "tx-gre-csum-segmentation",
>> -       [NETIF_F_GSO_IPIP_BIT] =         "tx-ipip-segmentation",
>> -       [NETIF_F_GSO_SIT_BIT] =          "tx-sit-segmentation",
>> +       [NETIF_F_GSO_IPXIP4_BIT] =       "tx-ipxip4-segmentation",
>> +       [NETIF_F_GSO_IPXIP6_BIT] =       "tx-ipxip6-segmentation",
>>         [NETIF_F_GSO_UDP_TUNNEL_BIT] =   "tx-udp_tnl-segmentation",
>>         [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
>>         [NETIF_F_GSO_PARTIAL_BIT] =      "tx-gso-partial",
>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>> index 7f08d45..25040b1 100644
>> --- a/net/ipv4/af_inet.c
>> +++ b/net/ipv4/af_inet.c
>> @@ -1483,7 +1483,7 @@ out_unlock:
>>  static int ipip_gro_complete(struct sk_buff *skb, int nhoff)
>>  {
>>         skb->encapsulation = 1;
>> -       skb_shinfo(skb)->gso_type |= SKB_GSO_IPIP;
>> +       skb_shinfo(skb)->gso_type |= SKB_GSO_IPXIP4;
>>         return inet_gro_complete(skb, nhoff);
>>  }
>>
>> diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
>> index 9282748..9783701 100644
>> --- a/net/ipv4/ipip.c
>> +++ b/net/ipv4/ipip.c
>> @@ -219,7 +219,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
>>         if (unlikely(skb->protocol != htons(ETH_P_IP)))
>>                 goto tx_error;
>>
>> -       if (iptunnel_handle_offloads(skb, SKB_GSO_IPIP))
>> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4))
>>                 goto tx_error;
>>
>>         skb_set_inner_ipproto(skb, IPPROTO_IPIP);
>> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
>> index 9ad743b..787e55f 100644
>> --- a/net/ipv6/ip6_offload.c
>> +++ b/net/ipv6/ip6_offload.c
>> @@ -86,7 +86,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>>         proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
>>
>>         if (skb->encapsulation &&
>> -           skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
>> +           skb_shinfo(skb)->gso_type & (SKB_GSO_IPXIP4 | SKB_GSO_IPXIP6))
>>                 udpfrag = proto == IPPROTO_UDP && encap;
>>         else
>>                 udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
>> @@ -294,7 +294,7 @@ out_unlock:
>>  static int sit_gro_complete(struct sk_buff *skb, int nhoff)
>>  {
>>         skb->encapsulation = 1;
>> -       skb_shinfo(skb)->gso_type |= SKB_GSO_SIT;
>> +       skb_shinfo(skb)->gso_type |= SKB_GSO_IPXIP4;
>>         return ipv6_gro_complete(skb, nhoff);
>>  }
>>
>> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
>> index a13d8c1..0a5a255 100644
>> --- a/net/ipv6/sit.c
>> +++ b/net/ipv6/sit.c
>> @@ -913,7 +913,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
>>                 goto tx_error;
>>         }
>>
>> -       if (iptunnel_handle_offloads(skb, SKB_GSO_SIT)) {
>> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4)) {
>>                 ip_rt_put(rt);
>>                 goto tx_error;
>>         }
>> @@ -1000,7 +1000,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
>>         struct ip_tunnel *tunnel = netdev_priv(dev);
>>         const struct iphdr  *tiph = &tunnel->parms.iph;
>>
>> -       if (iptunnel_handle_offloads(skb, SKB_GSO_IPIP))
>> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4))
>>                 goto tx_error;
>>
>>         skb_set_inner_ipproto(skb, IPPROTO_IPIP);
>> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
>> index 6d19d2e..f646ef6 100644
>> --- a/net/netfilter/ipvs/ip_vs_xmit.c
>> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
>> @@ -932,16 +932,9 @@ error:
>>
>>  static inline int __tun_gso_type_mask(int encaps_af, int orig_af)
>>  {
>> -       if (encaps_af == AF_INET) {
>> -               if (orig_af == AF_INET)
>> -                       return SKB_GSO_IPIP;
>> +       if (encaps_af == AF_INET)
>> +               return SKB_GSO_IPXIP4;
>>
>> -               return SKB_GSO_SIT;
>> -       }
>> -
>> -       /* GSO: we need to provide proper SKB_GSO_ value for IPv6:
>> -        * SKB_GSO_SIT/IPV6
>> -        */
>
> I wouldn't delete this comment until you actually have resolved the
> issue of providing a proper GSO value for IPv6.
>
>>         return 0;
>>  }
>>
>> --
>> 2.8.0.rc2
>>

^ permalink raw reply

* Re: [PATCH] netdev: enc28j60 kernel panic fix.
From: Francois Romieu @ 2016-05-06 20:35 UTC (permalink / raw)
  To: David Russell; +Cc: netdev
In-Reply-To: <CAEGHc+ymc3uY0C0WhjfEo5RoS-bQ7h7f6tbBQpAGEJ=D9rr6FA@mail.gmail.com>

(please don't top post)

David Russell <david@aprsworld.com> :
> I kind of thought my patch was at best incomplete.  When you state
> this change silences the bug but does not fix it, what are the
> implications of systems running this patch?  We have some production
> systems using this patch.  They reboot daily, but have been solid.

If my assumption is right it should drop an extra packet here and there.
No leak.

However transmit errors + transmit packets should still match the number
of times the driver calls enc28j60_send_packet (you would have to cook
your own stat to check the latter though).

> In addition, if we sent you a pi and the ethernet controller and a
> small but reasonable sum of money for your labor, would you be able to
> properly fix it ?

I'd rather see you testing my crap. :o)

Pi as multi-core (the expected race needs several cores or a netconsole
style transmit from an irq/bh context) ?

> Short of that, do you have any recommendations on quick overviews of
> the networking stack in the kernel and then documentation on the
> various flags and such?

A tad bit too high-level a question... Plain ctags + printk for a start ?

Does the patch below make a difference ?

Takes longer to crash counts as a difference.

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index 7066954..405fe3f 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -1170,7 +1170,8 @@ static void enc28j60_irq_work_handler(struct work_struct *work)
 				enc28j60_dump_tsv(priv, "Tx Done", tsv);
 			}
 			enc28j60_tx_clear(ndev, err);
-			locked_reg_bfclr(priv, EIR, EIR_TXIF);
+			locked_reg_bfclr(priv, EIR, EIR_TXIF | EIR_TXERIF);
+			intflags &= ~EIR_TXERIF;
 		}
 		/* TX Error handler */
 		if ((intflags & EIR_TXERIF) != 0) {
@@ -1190,6 +1191,7 @@ static void enc28j60_irq_work_handler(struct work_struct *work)
 			nolock_reg_bfclr(priv, ECON1, ECON1_TXRST);
 			nolock_txfifo_init(priv, TXSTART_INIT, TXEND_INIT);
 			mutex_unlock(&priv->lock);
+			locked_reg_bfclr(priv, EIR, EIR_TXIF | EIR_TXERIF);
 			/* Transmit Late collision check for retransmit */
 			if (TSV_GETBIT(tsv, TSV_TXLATECOLLISION)) {
 				if (netif_msg_tx_err(priv))
@@ -1203,7 +1205,6 @@ static void enc28j60_irq_work_handler(struct work_struct *work)
 					enc28j60_tx_clear(ndev, true);
 			} else
 				enc28j60_tx_clear(ndev, true);
-			locked_reg_bfclr(priv, EIR, EIR_TXERIF);
 		}
 		/* RX Error handler */
 		if ((intflags & EIR_RXERIF) != 0) {

^ permalink raw reply related

* Re: [PATCH v2 next-next 02/12] net: define gso types for IPx over IPv4 and IPv6
From: Alexander Duyck @ 2016-05-06 20:34 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <1462565096-4076043-3-git-send-email-tom@herbertland.com>

On Fri, May 6, 2016 at 1:04 PM, Tom Herbert <tom@herbertland.com> wrote:
> This patch defines two new GDO definitions SKB_GSO_IPXIP4 and
> SKB_GSO_IPXIP6 along with corresponding NETIF_F_GSO_IPXIP4 and
> NETIF_F_GSO_IPXIP6. These are used to described IP in IP
> tunnel and what the outer protocol is. The inner protocol
> can be deduced from other GSO types (e.g. SKB_GSO_TCPV4 and
> SKB_GSO_TCPV6). The GSO types of SKB_GSO_IPIP and SKB_GSO_SIT
> are removed (these are both instances of SKB_GSO_IPXIP4).
> SKB_GSO_IPXIP6 will be used when support for GSO with IP
> encapsulation over IPv6 is added.
>
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  |  5 ++---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c         |  4 ++--
>  drivers/net/ethernet/intel/i40e/i40e_main.c       |  3 +--
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c       |  3 +--
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c     |  3 +--
>  drivers/net/ethernet/intel/i40evf/i40evf_main.c   |  3 +--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |  3 +--
>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  3 +--
>  include/linux/netdev_features.h                   | 12 ++++++------
>  include/linux/netdevice.h                         |  4 ++--
>  include/linux/skbuff.h                            |  4 ++--
>  net/core/ethtool.c                                |  4 ++--
>  net/ipv4/af_inet.c                                |  2 +-
>  net/ipv4/ipip.c                                   |  2 +-
>  net/ipv6/ip6_offload.c                            |  4 ++--
>  net/ipv6/sit.c                                    |  4 ++--
>  net/netfilter/ipvs/ip_vs_xmit.c                   | 11 ++---------
>  17 files changed, 30 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index d465bd7..0a5b770 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -13259,12 +13259,11 @@ static int bnx2x_init_dev(struct bnx2x *bp, struct pci_dev *pdev,
>                 NETIF_F_RXHASH | NETIF_F_HW_VLAN_CTAG_TX;
>         if (!chip_is_e1x) {
>                 dev->hw_features |= NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL |
> -                                   NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT;
> +                                   NETIF_F_GSO_IPXIP4;
>                 dev->hw_enc_features =
>                         NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG |
>                         NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 |
> -                       NETIF_F_GSO_IPIP |
> -                       NETIF_F_GSO_SIT |
> +                       NETIF_F_GSO_IPXIP4 |
>                         NETIF_F_GSO_GRE | NETIF_F_GSO_UDP_TUNNEL;
>         }
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index fd85b6d..e449228 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -6218,7 +6218,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>         dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG |
>                            NETIF_F_TSO | NETIF_F_TSO6 |
>                            NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
> -                          NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
> +                          NETIF_F_GSO_IPXIP4 |
>                            NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
>                            NETIF_F_GSO_PARTIAL | NETIF_F_RXHASH |
>                            NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_GRO;
> @@ -6228,7 +6228,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>                         NETIF_F_TSO | NETIF_F_TSO6 |
>                         NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
>                         NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
> -                       NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
> +                       NETIF_F_GSO_IPXIP4;
>                         NETIF_F_GSO_PARTIAL;
>         dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM |
>                                     NETIF_F_GSO_GRE_CSUM;
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index f6da6b7..c2a4c10 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -9131,8 +9131,7 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)
>                                    NETIF_F_TSO6                 |
>                                    NETIF_F_GSO_GRE              |
>                                    NETIF_F_GSO_GRE_CSUM         |
> -                                  NETIF_F_GSO_IPIP             |
> -                                  NETIF_F_GSO_SIT              |
> +                                  NETIF_F_GSO_IPXIP4           |
>                                    NETIF_F_GSO_UDP_TUNNEL       |
>                                    NETIF_F_GSO_UDP_TUNNEL_CSUM  |
>                                    NETIF_F_GSO_PARTIAL          |

So for all the Intel drivers they support IPv6 and IPv4 outer header
tunnel types.  As such you can add SKB_GSO_IPXIP6 as well and you
should be able to see offloads without any issues.  This applies for
all the Intel drivers below and igb which should have a patch
available sometime soon as it is still in Jeff Kirsher's tree.

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index 2765d7e..4a3d60a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -2302,8 +2302,7 @@ static int i40e_tso(struct sk_buff *skb, u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
>
>         if (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE |
>                                          SKB_GSO_GRE_CSUM |
> -                                        SKB_GSO_IPIP |
> -                                        SKB_GSO_SIT |
> +                                        SKB_GSO_IPXIP4 |
>                                          SKB_GSO_UDP_TUNNEL |
>                                          SKB_GSO_UDP_TUNNEL_CSUM)) {
>                 if (!(skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) &&
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> index ede8dfc..aaccb7e 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
> @@ -1567,8 +1567,7 @@ static int i40e_tso(struct sk_buff *skb, u8 *hdr_len, u64 *cd_type_cmd_tso_mss)
>
>         if (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE |
>                                          SKB_GSO_GRE_CSUM |
> -                                        SKB_GSO_IPIP |
> -                                        SKB_GSO_SIT |
> +                                        SKB_GSO_IPXIP4 |
>                                          SKB_GSO_UDP_TUNNEL |
>                                          SKB_GSO_UDP_TUNNEL_CSUM)) {
>                 if (!(skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL) &&
> diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
> index 9f0bd7a..bfd0962 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
> @@ -2241,8 +2241,7 @@ int i40evf_process_config(struct i40evf_adapter *adapter)
>                                    NETIF_F_TSO6                 |
>                                    NETIF_F_GSO_GRE              |
>                                    NETIF_F_GSO_GRE_CSUM         |
> -                                  NETIF_F_GSO_IPIP             |
> -                                  NETIF_F_GSO_SIT              |
> +                                  NETIF_F_GSO_IPXIP4           |
>                                    NETIF_F_GSO_UDP_TUNNEL       |
>                                    NETIF_F_GSO_UDP_TUNNEL_CSUM  |
>                                    NETIF_F_GSO_PARTIAL          |
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index d08fbcf..28e3b5a 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -9482,8 +9482,7 @@ skip_sriov:
>
>  #define IXGBE_GSO_PARTIAL_FEATURES (NETIF_F_GSO_GRE | \
>                                     NETIF_F_GSO_GRE_CSUM | \
> -                                   NETIF_F_GSO_IPIP | \
> -                                   NETIF_F_GSO_SIT | \
> +                                   NETIF_F_GSO_IPXIP4 | \
>                                     NETIF_F_GSO_UDP_TUNNEL | \
>                                     NETIF_F_GSO_UDP_TUNNEL_CSUM)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 5e348b1..d86e511 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -4062,8 +4062,7 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
>  #define IXGBEVF_GSO_PARTIAL_FEATURES (NETIF_F_GSO_GRE | \
>                                       NETIF_F_GSO_GRE_CSUM | \
> -                                     NETIF_F_GSO_IPIP | \
> -                                     NETIF_F_GSO_SIT | \
> +                                     NETIF_F_GSO_IPXIP4 | \
>                                       NETIF_F_GSO_UDP_TUNNEL | \
>                                       NETIF_F_GSO_UDP_TUNNEL_CSUM)
>

So with GSO partial all of the igb/ixgbe parts should be able to
support any tunnel type you can throw at it.

> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index bc87362..aa7b240 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -44,8 +44,8 @@ enum {
>         NETIF_F_FSO_BIT,                /* ... FCoE segmentation */
>         NETIF_F_GSO_GRE_BIT,            /* ... GRE with TSO */
>         NETIF_F_GSO_GRE_CSUM_BIT,       /* ... GRE with csum with TSO */
> -       NETIF_F_GSO_IPIP_BIT,           /* ... IPIP tunnel with TSO */
> -       NETIF_F_GSO_SIT_BIT,            /* ... SIT tunnel with TSO */
> +       NETIF_F_GSO_IPXIP4_BIT,         /* ... IP4 or IP6 over IP4 with TSO */
> +       NETIF_F_GSO_IPXIP6_BIT,         /* ... IP4 or IP6 over IP6 with TSO */
>         NETIF_F_GSO_UDP_TUNNEL_BIT,     /* ... UDP TUNNEL with TSO */
>         NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT,/* ... UDP TUNNEL with TSO & CSUM */
>         NETIF_F_GSO_PARTIAL_BIT,        /* ... Only segment inner-most L4
> @@ -121,8 +121,8 @@ enum {
>  #define NETIF_F_RXALL          __NETIF_F(RXALL)
>  #define NETIF_F_GSO_GRE                __NETIF_F(GSO_GRE)
>  #define NETIF_F_GSO_GRE_CSUM   __NETIF_F(GSO_GRE_CSUM)
> -#define NETIF_F_GSO_IPIP       __NETIF_F(GSO_IPIP)
> -#define NETIF_F_GSO_SIT                __NETIF_F(GSO_SIT)
> +#define NETIF_F_GSO_IPXIP4     __NETIF_F(GSO_IPXIP4)
> +#define NETIF_F_GSO_IPXIP6     __NETIF_F(GSO_IPXIP6)
>  #define NETIF_F_GSO_UDP_TUNNEL __NETIF_F(GSO_UDP_TUNNEL)
>  #define NETIF_F_GSO_UDP_TUNNEL_CSUM __NETIF_F(GSO_UDP_TUNNEL_CSUM)
>  #define NETIF_F_TSO_MANGLEID   __NETIF_F(TSO_MANGLEID)
> @@ -200,8 +200,8 @@ enum {
>
>  #define NETIF_F_GSO_ENCAP_ALL  (NETIF_F_GSO_GRE |                      \
>                                  NETIF_F_GSO_GRE_CSUM |                 \
> -                                NETIF_F_GSO_IPIP |                     \
> -                                NETIF_F_GSO_SIT |                      \
> +                                NETIF_F_GSO_IPXIP4 |                   \
> +                                NETIF_F_GSO_IPXIP6 |                   \
>                                  NETIF_F_GSO_UDP_TUNNEL |               \
>                                  NETIF_F_GSO_UDP_TUNNEL_CSUM)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 63580e6..8b1eedc 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4005,8 +4005,8 @@ static inline bool net_gso_ok(netdev_features_t features, int gso_type)
>         BUILD_BUG_ON(SKB_GSO_FCOE    != (NETIF_F_FSO >> NETIF_F_GSO_SHIFT));
>         BUILD_BUG_ON(SKB_GSO_GRE     != (NETIF_F_GSO_GRE >> NETIF_F_GSO_SHIFT));
>         BUILD_BUG_ON(SKB_GSO_GRE_CSUM != (NETIF_F_GSO_GRE_CSUM >> NETIF_F_GSO_SHIFT));
> -       BUILD_BUG_ON(SKB_GSO_IPIP    != (NETIF_F_GSO_IPIP >> NETIF_F_GSO_SHIFT));
> -       BUILD_BUG_ON(SKB_GSO_SIT     != (NETIF_F_GSO_SIT >> NETIF_F_GSO_SHIFT));
> +       BUILD_BUG_ON(SKB_GSO_IPXIP4  != (NETIF_F_GSO_IPXIP4 >> NETIF_F_GSO_SHIFT));
> +       BUILD_BUG_ON(SKB_GSO_IPXIP6  != (NETIF_F_GSO_IPXIP6 >> NETIF_F_GSO_SHIFT));
>         BUILD_BUG_ON(SKB_GSO_UDP_TUNNEL != (NETIF_F_GSO_UDP_TUNNEL >> NETIF_F_GSO_SHIFT));
>         BUILD_BUG_ON(SKB_GSO_UDP_TUNNEL_CSUM != (NETIF_F_GSO_UDP_TUNNEL_CSUM >> NETIF_F_GSO_SHIFT));
>         BUILD_BUG_ON(SKB_GSO_PARTIAL != (NETIF_F_GSO_PARTIAL >> NETIF_F_GSO_SHIFT));
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index c413c58..65968a9 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -471,9 +471,9 @@ enum {
>
>         SKB_GSO_GRE_CSUM = 1 << 8,
>
> -       SKB_GSO_IPIP = 1 << 9,
> +       SKB_GSO_IPXIP4 = 1 << 9,
>
> -       SKB_GSO_SIT = 1 << 10,
> +       SKB_GSO_IPXIP6 = 1 << 10,
>
>         SKB_GSO_UDP_TUNNEL = 1 << 11,
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index bdb4013..f403481 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -84,8 +84,8 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
>         [NETIF_F_FSO_BIT] =              "tx-fcoe-segmentation",
>         [NETIF_F_GSO_GRE_BIT] =          "tx-gre-segmentation",
>         [NETIF_F_GSO_GRE_CSUM_BIT] =     "tx-gre-csum-segmentation",
> -       [NETIF_F_GSO_IPIP_BIT] =         "tx-ipip-segmentation",
> -       [NETIF_F_GSO_SIT_BIT] =          "tx-sit-segmentation",
> +       [NETIF_F_GSO_IPXIP4_BIT] =       "tx-ipxip4-segmentation",
> +       [NETIF_F_GSO_IPXIP6_BIT] =       "tx-ipxip6-segmentation",
>         [NETIF_F_GSO_UDP_TUNNEL_BIT] =   "tx-udp_tnl-segmentation",
>         [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
>         [NETIF_F_GSO_PARTIAL_BIT] =      "tx-gso-partial",
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 7f08d45..25040b1 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1483,7 +1483,7 @@ out_unlock:
>  static int ipip_gro_complete(struct sk_buff *skb, int nhoff)
>  {
>         skb->encapsulation = 1;
> -       skb_shinfo(skb)->gso_type |= SKB_GSO_IPIP;
> +       skb_shinfo(skb)->gso_type |= SKB_GSO_IPXIP4;
>         return inet_gro_complete(skb, nhoff);
>  }
>
> diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
> index 9282748..9783701 100644
> --- a/net/ipv4/ipip.c
> +++ b/net/ipv4/ipip.c
> @@ -219,7 +219,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
>         if (unlikely(skb->protocol != htons(ETH_P_IP)))
>                 goto tx_error;
>
> -       if (iptunnel_handle_offloads(skb, SKB_GSO_IPIP))
> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4))
>                 goto tx_error;
>
>         skb_set_inner_ipproto(skb, IPPROTO_IPIP);
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 9ad743b..787e55f 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -86,7 +86,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>         proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
>
>         if (skb->encapsulation &&
> -           skb_shinfo(skb)->gso_type & (SKB_GSO_SIT|SKB_GSO_IPIP))
> +           skb_shinfo(skb)->gso_type & (SKB_GSO_IPXIP4 | SKB_GSO_IPXIP6))
>                 udpfrag = proto == IPPROTO_UDP && encap;
>         else
>                 udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
> @@ -294,7 +294,7 @@ out_unlock:
>  static int sit_gro_complete(struct sk_buff *skb, int nhoff)
>  {
>         skb->encapsulation = 1;
> -       skb_shinfo(skb)->gso_type |= SKB_GSO_SIT;
> +       skb_shinfo(skb)->gso_type |= SKB_GSO_IPXIP4;
>         return ipv6_gro_complete(skb, nhoff);
>  }
>
> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index a13d8c1..0a5a255 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -913,7 +913,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
>                 goto tx_error;
>         }
>
> -       if (iptunnel_handle_offloads(skb, SKB_GSO_SIT)) {
> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4)) {
>                 ip_rt_put(rt);
>                 goto tx_error;
>         }
> @@ -1000,7 +1000,7 @@ static netdev_tx_t ipip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
>         struct ip_tunnel *tunnel = netdev_priv(dev);
>         const struct iphdr  *tiph = &tunnel->parms.iph;
>
> -       if (iptunnel_handle_offloads(skb, SKB_GSO_IPIP))
> +       if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4))
>                 goto tx_error;
>
>         skb_set_inner_ipproto(skb, IPPROTO_IPIP);
> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
> index 6d19d2e..f646ef6 100644
> --- a/net/netfilter/ipvs/ip_vs_xmit.c
> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> @@ -932,16 +932,9 @@ error:
>
>  static inline int __tun_gso_type_mask(int encaps_af, int orig_af)
>  {
> -       if (encaps_af == AF_INET) {
> -               if (orig_af == AF_INET)
> -                       return SKB_GSO_IPIP;
> +       if (encaps_af == AF_INET)
> +               return SKB_GSO_IPXIP4;
>
> -               return SKB_GSO_SIT;
> -       }
> -
> -       /* GSO: we need to provide proper SKB_GSO_ value for IPv6:
> -        * SKB_GSO_SIT/IPV6
> -        */

I wouldn't delete this comment until you actually have resolved the
issue of providing a proper GSO value for IPv6.

>         return 0;
>  }
>
> --
> 2.8.0.rc2
>

^ permalink raw reply

* Re: [PATCH v2 next-next 04/12] udp: Don't set skb->encapsulation with RCO
From: Alexander Duyck @ 2016-05-06 20:23 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <1462565096-4076043-5-git-send-email-tom@herbertland.com>

On Fri, May 6, 2016 at 1:04 PM, Tom Herbert <tom@herbertland.com> wrote:
> When RCO is in effect we want to ensure that the outer checksum is
> properly offloaded. Don't set skb->encapsulation in this case to
> ensure that checksum offload is later considered for hw_features
> instead of hw_enc_features.
>
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
>  net/ipv4/udp_offload.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index b556ef6..92a9222 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -94,11 +94,12 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
>         do {
>                 unsigned int len;
>
> -               if (remcsum)
> +               if (remcsum) {
>                         skb->ip_summed = CHECKSUM_NONE;
> -
> -               /* Set up inner headers if we are offloading inner checksum */
> -               if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +               } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +                       /* Set up inner headers if we are offloading inner
> +                        * checksum
> +                        */
>                         skb_reset_inner_headers(skb);
>                         skb->encapsulation = 1;
>                 }

You can just drop this patch.  It doesn't actually change anything
behaviorwise.  If remcsum is set then skb->ip_summed is set to
CHECKSUM_NONE so it isn't going to hit the CHECKSUM_PARTIAL code so
you don't need the else.

- Alex

^ permalink raw reply

* [patch net-next] mlxsw: spectrum: Fix ordering in mlxsw_sp_fini
From: Jiri Pirko @ 2016-05-06 20:20 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg, ogerlitz

From: Jiri Pirko <jiri@mellanox.com>

Fixes: 0f433fa0ec ("mlxsw: spectrum_buffers: Implement shared buffer configuration")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 4758516..4a72737 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -2449,8 +2449,8 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
 
-	mlxsw_sp_buffers_fini(mlxsw_sp);
 	mlxsw_sp_switchdev_fini(mlxsw_sp);
+	mlxsw_sp_buffers_fini(mlxsw_sp);
 	mlxsw_sp_traps_fini(mlxsw_sp);
 	mlxsw_sp_event_unregister(mlxsw_sp, MLXSW_TRAP_ID_PUDE);
 	mlxsw_sp_ports_remove(mlxsw_sp);
-- 
2.5.5

^ permalink raw reply related

* Re: [PATCH net v3 2/2] udp_offload: Set encapsulation before inner completes.
From: Tom Herbert @ 2016-05-06 20:20 UTC (permalink / raw)
  To: David Miller
  Cc: Jarno Rajahalme, Linux Kernel Network Developers, Alexander Duyck
In-Reply-To: <20160506.153412.1756777490005831525.davem@davemloft.net>

On Fri, May 6, 2016 at 12:34 PM, David Miller <davem@davemloft.net> wrote:
> From: Jarno Rajahalme <jarno@ovn.org>
> Date: Tue,  3 May 2016 16:10:21 -0700
>
>> UDP tunnel segmentation code relies on the inner offsets being set for
>> an UDP tunnel GSO packet, but the inner *_complete() functions will
>> set the inner offsets only if 'encapsulation' is set before calling
>> them.  Currently, udp_gro_complete() sets 'encapsulation' only after
>> the inner *_complete() functions are done.  This causes the inner
>> offsets having invalid values after udp_gro_complete() returns, which
>> in turn will make it impossible to properly segment the packet in case
>> it needs to be forwarded, which would be visible to the user either as
>> invalid packets being sent or as packet loss.
>>
>> This patch fixes this by setting skb's 'encapsulation' in
>> udp_gro_complete() before calling into the inner complete functions,
>> and by making each possible UDP tunnel gro_complete() callback set the
>> inner_mac_header to the beginning of the tunnel payload.
>>
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>> ---
>> v3: Added setting inner_mac_header from all possible callbacks to cover
>>     cases where there is no inner mac header.
>
> Alex and Tom, can you please review this new version since you guys had
> so much feedback for v2?
>

I'm okay with the patch.Clarifying exactly what skb->encaspulation
means is future work.

> THanks.

^ permalink raw reply

* [patch net 2/2] mlxsw: spectrum: Add missing rollback in flood configuration
From: Jiri Pirko @ 2016-05-06 20:18 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg, ogerlitz
In-Reply-To: <1462565920-1959-1-git-send-email-jiri@resnulli.us>

From: Ido Schimmel <idosch@mellanox.com>

When we fail to set the flooding configuration for the broadcast and
unregistered multicast traffic, we should revert the flooding
configuration of the unknown unicast traffic.

Fixes: 0293038e0c36 ("mlxsw: spectrum: Add support for flood control")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index e1c74ef..9cd6f47 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -214,7 +214,15 @@ static int __mlxsw_sp_port_flood_set(struct mlxsw_sp_port *mlxsw_sp_port,
 	mlxsw_reg_sftr_pack(sftr_pl, MLXSW_SP_FLOOD_TABLE_BM, idx_begin,
 			    table_type, range, local_port, set);
 	err = mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(sftr), sftr_pl);
+	if (err)
+		goto err_flood_bm_set;
+	else
+		goto buffer_out;
 
+err_flood_bm_set:
+	mlxsw_reg_sftr_pack(sftr_pl, MLXSW_SP_FLOOD_TABLE_UC, idx_begin,
+			    table_type, range, local_port, !set);
+	mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(sftr), sftr_pl);
 buffer_out:
 	kfree(sftr_pl);
 	return err;
-- 
2.5.5

^ permalink raw reply related

* [patch net 1/2] mlxsw: spectrum: Fix rollback order in LAG join failure
From: Jiri Pirko @ 2016-05-06 20:18 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg, ogerlitz
In-Reply-To: <1462565920-1959-1-git-send-email-jiri@resnulli.us>

From: Ido Schimmel <idosch@mellanox.com>

Make the leave procedure in the error path symmetric to the join
procedure and first remove the port from the collector before
potentially destroying the LAG.

Fixes: 0d65fc13042f ("mlxsw: spectrum: Implement LAG port join/leave")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 4afbc3e..668b2f4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -2541,11 +2541,11 @@ static int mlxsw_sp_port_lag_join(struct mlxsw_sp_port *mlxsw_sp_port,
 	lag->ref_count++;
 	return 0;
 
+err_col_port_enable:
+	mlxsw_sp_lag_col_port_remove(mlxsw_sp_port, lag_id);
 err_col_port_add:
 	if (!lag->ref_count)
 		mlxsw_sp_lag_destroy(mlxsw_sp, lag_id);
-err_col_port_enable:
-	mlxsw_sp_lag_col_port_remove(mlxsw_sp_port, lag_id);
 	return err;
 }
 
-- 
2.5.5

^ permalink raw reply related

* [patch net 0/2] mlxsw: Couple of fixes
From: Jiri Pirko @ 2016-05-06 20:18 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, eladr, yotamg, ogerlitz

From: Jiri Pirko <jiri@mellanox.com>

Ido Schimmel (2):
  mlxsw: spectrum: Fix rollback order in LAG join failure
  mlxsw: spectrum: Add missing rollback in flood configuration

 drivers/net/ethernet/mellanox/mlxsw/spectrum.c           | 4 ++--
 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.5.5

^ permalink raw reply

* Re: [patch net 0/3] mlxsw: Couple of fixes
From: David Miller @ 2016-05-06 20:15 UTC (permalink / raw)
  To: jiri; +Cc: netdev, idosch, eladr, yotamg, ogerlitz
In-Reply-To: <20160506201349.GA1973@nanopsycho.orion>

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 6 May 2016 22:13:49 +0200

> Fri, May 06, 2016 at 10:00:32PM CEST, davem@davemloft.net wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Date: Fri,  6 May 2016 11:17:21 +0200
>>
>>> From: Jiri Pirko <jiri@mellanox.com>
>>> 
>>> Ido Schimmel (2):
>>>   mlxsw: spectrum: Fix rollback order in LAG join failure
>>>   mlxsw: spectrum: Add missing rollback in flood configuration
>>> 
>>> Jiri Pirko (1):
>>>   mlxsw: spectrum: Fix ordering in mlxsw_sp_fini
>>
>>What tree is this for?  Because on 'net' this makes the build fail.
>>
>>drivers/net/ethernet/mellanox/mlxsw/spectrum.c: In function ‘mlxsw_sp_fini’:
>>drivers/net/ethernet/mellanox/mlxsw/spectrum.c:2162:2: error: implicit declaration of function ‘mlxsw_sp_buffers_fini’ [-Werror=implicit-function-declaration]
>>
> 
> Oh. Ido's patches are for -net, mine if for -net-next. I did't realize,
> sorry. Do you want me to repost?

Yes, please post them properly.

Thanks.

^ permalink raw reply

* Re: [patch net 0/3] mlxsw: Couple of fixes
From: Jiri Pirko @ 2016-05-06 20:13 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, idosch, eladr, yotamg, ogerlitz
In-Reply-To: <20160506.160032.988131639794984436.davem@davemloft.net>

Fri, May 06, 2016 at 10:00:32PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri,  6 May 2016 11:17:21 +0200
>
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> Ido Schimmel (2):
>>   mlxsw: spectrum: Fix rollback order in LAG join failure
>>   mlxsw: spectrum: Add missing rollback in flood configuration
>> 
>> Jiri Pirko (1):
>>   mlxsw: spectrum: Fix ordering in mlxsw_sp_fini
>
>What tree is this for?  Because on 'net' this makes the build fail.
>
>drivers/net/ethernet/mellanox/mlxsw/spectrum.c: In function ‘mlxsw_sp_fini’:
>drivers/net/ethernet/mellanox/mlxsw/spectrum.c:2162:2: error: implicit declaration of function ‘mlxsw_sp_buffers_fini’ [-Werror=implicit-function-declaration]
>

Oh. Ido's patches are for -net, mine if for -net-next. I did't realize,
sorry. Do you want me to repost?

^ permalink raw reply

* Re: [PATCH 2/3] net/mlx5e: make VXLAN support conditional
From: Alexander Duyck @ 2016-05-06 20:12 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Saeed Mahameed, Matan Barak, Leon Romanovsky, Saeed Mahameed,
	Linux Netdev List, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David S. Miller, Matthew Finlay, Richard Cochran, Amir Vadai,
	Haggai Abramonvsky, Maor Gottlieb, Or Gerlitz, Majd Dibbiny,
	Achiad Shochat, Tariq Toukan, Gal Pressman
In-Reply-To: <4306900.EI0LQtX7kx@wuerfel>

On Thu, May 5, 2016 at 11:09 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday 05 May 2016 19:44:36 Saeed Mahameed wrote:
>> On Wed, May 4, 2016 at 3:31 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> > VXLAN can be disabled at compile-time or it can be a loadable
>> > module while mlx5 is built-in, which leads to a link error:
>> >
>> > drivers/net/built-in.o: In function `mlx5e_create_netdev':
>> > ntb_netdev.c:(.text+0x106de4): undefined reference to `vxlan_get_rx_port'
>> >
>> > This avoids the link error and makes the vxlan code optional,
>> > like the other ethernet drivers do as well.
>> >
>> > Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>> > Link: https://patchwork.ozlabs.org/patch/589296/
>> > Fixes: b3f63c3d5e2c ("net/mlx5e: Add netdev support for VXLAN tunneling")
>> > ---
>> > I sent it originally on Feb 26 2016, but misread Saeed Mahameed's
>> > reply as saying that he'd fix it up himself. The new version
>> > should address the original comment.
>> > ---
>>
>> Hi Arnd,
>>
>> I didn't post a fix up since it is not needed anymore, see
>> b7aade15485a ('vxlan: break dependency with netdev drivers') in
>> net-next.
>>
>> The new issue is introduced due to : "net/mlx5: Kconfig: Fix
>> MLX5_EN/VXLAN build issue" which was merged from net tree.
>>
>> Dave shouldn't have merged it into net-next, I explicitly asked him
>> that in the cover letter.  Maybe he missed it.
>>
>> I just checked and It is sufficient to only take the revert patch:
>> [PATCH 1/3] Revert "net/mlx5: Kconfig: Fix MLX5_EN/VXLAN build issue"
>> to net-next.
>>
>> Can you please confirm that with only the revert patch, you don't see
>> the issue ?
>
> Yes, it works, but not it is different from all the other drivers
> (MLX4, BENET, IXGBE, I40E, FM10K, QLCNIC, and QEDE). If the 'select
> VXLAN' is not the preferred way to handle this, we should change
> the other ones the same way, right?
>
> For reference, I've tried it out on the MLX4 driver, and it does
> seem nicer that way, see below.
>
>         Arnd
> ---
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> in case someone wants to pick up that patch and do the other
> ones as well.
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
> index 9ca3734ebb6b..88fff4484200 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
> +++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
> @@ -6,6 +6,7 @@ config MLX4_EN
>         tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
>         depends on MAY_USE_DEVLINK
>         depends on PCI
> +       select VXLAN
>         select MLX4_CORE
>         select PTP_1588_CLOCK
>         ---help---

This piece is unnecessary and unwanted.  We just recently added the
ability to load the modules without the need for VXLAN.  Lets not take
that in the wrong direction by having the drivers select a module they
don't have to have.

The rest of this code is probably fine.  After the dependency was
broken via b7aade15485a ('vxlan: break dependency with netdev
drivers') you could probably go through and just pull all the VXLAN
ifdefs from all the drivers since I don't think there is anything that
explicitly relies on that module anymore as the only export still
hanging around is vxlan_dev_create and I don't think any Ethernet
drivers are directly spawning VXLAN interfaces.

> @@ -24,13 +25,6 @@ config MLX4_EN_DCB
>
>           If unsure, set to Y
>
> -config MLX4_EN_VXLAN
> -       bool "VXLAN offloads Support"
> -       default y
> -       depends on MLX4_EN && VXLAN && !(MLX4_EN=y && VXLAN=m)
> -       ---help---
> -         Say Y here if you want to use VXLAN offloads in the driver.
> -
>  config MLX4_CORE
>         tristate
>         depends on PCI
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 6f28ac58251c..ad887c425f2d 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -1691,10 +1691,8 @@ int mlx4_en_start_port(struct net_device *dev)
>         /* Schedule multicast task to populate multicast list */
>         queue_work(mdev->workqueue, &priv->rx_mode_task);
>
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         if (priv->mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN)
>                 vxlan_get_rx_port(dev);
> -#endif
>         priv->port_up = true;
>         netif_tx_start_all_queues(dev);
>         netif_device_attach(dev);
> @@ -2337,7 +2335,6 @@ static int mlx4_en_get_phys_port_id(struct net_device *dev,
>         return 0;
>  }
>
> -#ifdef CONFIG_MLX4_EN_VXLAN
>  static void mlx4_en_add_vxlan_offloads(struct work_struct *work)
>  {
>         int ret;
> @@ -2448,7 +2445,6 @@ static netdev_features_t mlx4_en_features_check(struct sk_buff *skb,
>
>         return features;
>  }
> -#endif
>
>  static int mlx4_en_set_tx_maxrate(struct net_device *dev, int queue_index, u32 maxrate)
>  {
> @@ -2501,11 +2497,9 @@ static const struct net_device_ops mlx4_netdev_ops = {
>         .ndo_rx_flow_steer      = mlx4_en_filter_rfs,
>  #endif
>         .ndo_get_phys_port_id   = mlx4_en_get_phys_port_id,
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         .ndo_add_vxlan_port     = mlx4_en_add_vxlan_port,
>         .ndo_del_vxlan_port     = mlx4_en_del_vxlan_port,
>         .ndo_features_check     = mlx4_en_features_check,
> -#endif
>         .ndo_set_tx_maxrate     = mlx4_en_set_tx_maxrate,
>  };
>
> @@ -2539,11 +2533,9 @@ static const struct net_device_ops mlx4_netdev_ops_master = {
>         .ndo_rx_flow_steer      = mlx4_en_filter_rfs,
>  #endif
>         .ndo_get_phys_port_id   = mlx4_en_get_phys_port_id,
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         .ndo_add_vxlan_port     = mlx4_en_add_vxlan_port,
>         .ndo_del_vxlan_port     = mlx4_en_del_vxlan_port,
>         .ndo_features_check     = mlx4_en_features_check,
> -#endif
>         .ndo_set_tx_maxrate     = mlx4_en_set_tx_maxrate,
>  };
>
> @@ -2834,10 +2826,8 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
>         INIT_WORK(&priv->linkstate_task, mlx4_en_linkstate);
>         INIT_DELAYED_WORK(&priv->stats_task, mlx4_en_do_get_stats);
>         INIT_DELAYED_WORK(&priv->service_task, mlx4_en_service_task);
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         INIT_WORK(&priv->vxlan_add_task, mlx4_en_add_vxlan_offloads);
>         INIT_WORK(&priv->vxlan_del_task, mlx4_en_del_vxlan_offloads);
> -#endif
>  #ifdef CONFIG_RFS_ACCEL
>         INIT_LIST_HEAD(&priv->filters);
>         spin_lock_init(&priv->filters_lock);
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> index 63b1aeae2c03..cbab8741d60f 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> @@ -546,10 +546,8 @@ struct mlx4_en_priv {
>         struct work_struct linkstate_task;
>         struct delayed_work stats_task;
>         struct delayed_work service_task;
> -#ifdef CONFIG_MLX4_EN_VXLAN
>         struct work_struct vxlan_add_task;
>         struct work_struct vxlan_del_task;
> -#endif
>         struct mlx4_en_perf_stats pstats;
>         struct mlx4_en_pkt_stats pkstats;
>         struct mlx4_en_counter_stats pf_stats;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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

* [PATCH v2 next-next 05/12] fou: Call setup_udp_tunnel_sock
From: Tom Herbert @ 2016-05-06 20:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1462565096-4076043-1-git-send-email-tom@herbertland.com>

Use helper function to set up UDP tunnel related information for a fou
socket.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/ipv4/fou.c | 50 ++++++++++++++++----------------------------------
 1 file changed, 16 insertions(+), 34 deletions(-)

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 7ac5ec8..a8b5cbf 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -446,31 +446,13 @@ static void fou_release(struct fou *fou)
 	kfree_rcu(fou, rcu);
 }
 
-static int fou_encap_init(struct sock *sk, struct fou *fou, struct fou_cfg *cfg)
-{
-	udp_sk(sk)->encap_rcv = fou_udp_recv;
-	udp_sk(sk)->gro_receive = fou_gro_receive;
-	udp_sk(sk)->gro_complete = fou_gro_complete;
-	fou_from_sock(sk)->protocol = cfg->protocol;
-
-	return 0;
-}
-
-static int gue_encap_init(struct sock *sk, struct fou *fou, struct fou_cfg *cfg)
-{
-	udp_sk(sk)->encap_rcv = gue_udp_recv;
-	udp_sk(sk)->gro_receive = gue_gro_receive;
-	udp_sk(sk)->gro_complete = gue_gro_complete;
-
-	return 0;
-}
-
 static int fou_create(struct net *net, struct fou_cfg *cfg,
 		      struct socket **sockp)
 {
 	struct socket *sock = NULL;
 	struct fou *fou = NULL;
 	struct sock *sk;
+	struct udp_tunnel_sock_cfg tunnel_cfg;
 	int err;
 
 	/* Open UDP socket */
@@ -489,33 +471,33 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
 
 	fou->flags = cfg->flags;
 	fou->port = cfg->udp_config.local_udp_port;
+	fou->type = cfg->type;
+	fou->sock = sock;
+
+	memset(&tunnel_cfg, 0, sizeof(tunnel_cfg));
+	tunnel_cfg.encap_type = 1;
+	tunnel_cfg.sk_user_data = fou;
+	tunnel_cfg.encap_destroy = NULL;
 
 	/* Initial for fou type */
 	switch (cfg->type) {
 	case FOU_ENCAP_DIRECT:
-		err = fou_encap_init(sk, fou, cfg);
-		if (err)
-			goto error;
+		tunnel_cfg.encap_rcv = fou_udp_recv;
+		tunnel_cfg.gro_receive = fou_gro_receive;
+		tunnel_cfg.gro_complete = fou_gro_complete;
+		fou->protocol = cfg->protocol;
 		break;
 	case FOU_ENCAP_GUE:
-		err = gue_encap_init(sk, fou, cfg);
-		if (err)
-			goto error;
+		tunnel_cfg.encap_rcv = gue_udp_recv;
+		tunnel_cfg.gro_receive = gue_gro_receive;
+		tunnel_cfg.gro_complete = gue_gro_complete;
 		break;
 	default:
 		err = -EINVAL;
 		goto error;
 	}
 
-	fou->type = cfg->type;
-
-	udp_sk(sk)->encap_type = 1;
-	udp_encap_enable();
-
-	sk->sk_user_data = fou;
-	fou->sock = sock;
-
-	inet_inc_convert_csum(sk);
+	setup_udp_tunnel_sock(net, sock, &tunnel_cfg);
 
 	sk->sk_allocation = GFP_ATOMIC;
 
-- 
2.8.0.rc2

^ permalink raw reply related

* [PATCH v2 next-next 12/12] ip6_gre: Add support for fou/gue encapsulation
From: Tom Herbert @ 2016-05-06 20:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1462565096-4076043-1-git-send-email-tom@herbertland.com>

Add netlink and setup for encapsulation

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/ipv6/ip6_gre.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 70a1f72..ed5ddcc 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1027,6 +1027,8 @@ static int ip6gre_tunnel_init_common(struct net_device *dev)
 
 	tunnel->tun_hlen = gre_calc_hlen(tunnel->parms.o_flags);
 
+	tunnel->hlen = tunnel->tun_hlen + tunnel->encap_hlen;
+
 	t_hlen = tunnel->hlen + sizeof(struct ipv6hdr);
 
 	dev->needed_headroom	= LL_MAX_HEADER + t_hlen + 4;
@@ -1293,15 +1295,57 @@ static void ip6gre_tap_setup(struct net_device *dev)
 	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
 }
 
+static bool ip6gre_netlink_encap_parms(struct nlattr *data[],
+				       struct ip_tunnel_encap *ipencap)
+{
+	bool ret = false;
+
+	memset(ipencap, 0, sizeof(*ipencap));
+
+	if (!data)
+		return ret;
+
+	if (data[IFLA_GRE_ENCAP_TYPE]) {
+		ret = true;
+		ipencap->type = nla_get_u16(data[IFLA_GRE_ENCAP_TYPE]);
+	}
+
+	if (data[IFLA_GRE_ENCAP_FLAGS]) {
+		ret = true;
+		ipencap->flags = nla_get_u16(data[IFLA_GRE_ENCAP_FLAGS]);
+	}
+
+	if (data[IFLA_GRE_ENCAP_SPORT]) {
+		ret = true;
+		ipencap->sport = nla_get_be16(data[IFLA_GRE_ENCAP_SPORT]);
+	}
+
+	if (data[IFLA_GRE_ENCAP_DPORT]) {
+		ret = true;
+		ipencap->dport = nla_get_be16(data[IFLA_GRE_ENCAP_DPORT]);
+	}
+
+	return ret;
+}
+
 static int ip6gre_newlink(struct net *src_net, struct net_device *dev,
 	struct nlattr *tb[], struct nlattr *data[])
 {
 	struct ip6_tnl *nt;
 	struct net *net = dev_net(dev);
 	struct ip6gre_net *ign = net_generic(net, ip6gre_net_id);
+	struct ip_tunnel_encap ipencap;
 	int err;
 
 	nt = netdev_priv(dev);
+
+	if (ip6gre_netlink_encap_parms(data, &ipencap)) {
+		int err = ip6_tnl_encap_setup(nt, &ipencap);
+
+		if (err < 0)
+			return err;
+	}
+
 	ip6gre_netlink_parms(data, &nt->parms);
 
 	if (ip6gre_tunnel_find(net, &nt->parms, dev->type))
@@ -1348,10 +1392,18 @@ static int ip6gre_changelink(struct net_device *dev, struct nlattr *tb[],
 	struct net *net = nt->net;
 	struct ip6gre_net *ign = net_generic(net, ip6gre_net_id);
 	struct __ip6_tnl_parm p;
+	struct ip_tunnel_encap ipencap;
 
 	if (dev == ign->fb_tunnel_dev)
 		return -EINVAL;
 
+	if (ip6gre_netlink_encap_parms(data, &ipencap)) {
+		int err = ip6_tnl_encap_setup(nt, &ipencap);
+
+		if (err < 0)
+			return err;
+	}
+
 	ip6gre_netlink_parms(data, &p);
 
 	t = ip6gre_tunnel_locate(net, &p, 0);
@@ -1405,6 +1457,14 @@ static size_t ip6gre_get_size(const struct net_device *dev)
 		nla_total_size(4) +
 		/* IFLA_GRE_FLAGS */
 		nla_total_size(4) +
+		/* IFLA_GRE_ENCAP_TYPE */
+		nla_total_size(2) +
+		/* IFLA_GRE_ENCAP_FLAGS */
+		nla_total_size(2) +
+		/* IFLA_GRE_ENCAP_SPORT */
+		nla_total_size(2) +
+		/* IFLA_GRE_ENCAP_DPORT */
+		nla_total_size(2) +
 		0;
 }
 
@@ -1428,6 +1488,17 @@ static int ip6gre_fill_info(struct sk_buff *skb, const struct net_device *dev)
 	    nla_put_be32(skb, IFLA_GRE_FLOWINFO, p->flowinfo) ||
 	    nla_put_u32(skb, IFLA_GRE_FLAGS, p->flags))
 		goto nla_put_failure;
+
+	if (nla_put_u16(skb, IFLA_GRE_ENCAP_TYPE,
+			t->encap.type) ||
+	    nla_put_be16(skb, IFLA_GRE_ENCAP_SPORT,
+			 t->encap.sport) ||
+	    nla_put_be16(skb, IFLA_GRE_ENCAP_DPORT,
+			 t->encap.dport) ||
+	    nla_put_u16(skb, IFLA_GRE_ENCAP_FLAGS,
+			t->encap.flags))
+		goto nla_put_failure;
+
 	return 0;
 
 nla_put_failure:
@@ -1446,6 +1517,10 @@ static const struct nla_policy ip6gre_policy[IFLA_GRE_MAX + 1] = {
 	[IFLA_GRE_ENCAP_LIMIT] = { .type = NLA_U8 },
 	[IFLA_GRE_FLOWINFO]    = { .type = NLA_U32 },
 	[IFLA_GRE_FLAGS]       = { .type = NLA_U32 },
+	[IFLA_GRE_ENCAP_TYPE]   = { .type = NLA_U16 },
+	[IFLA_GRE_ENCAP_FLAGS]  = { .type = NLA_U16 },
+	[IFLA_GRE_ENCAP_SPORT]  = { .type = NLA_U16 },
+	[IFLA_GRE_ENCAP_DPORT]  = { .type = NLA_U16 },
 };
 
 static struct rtnl_link_ops ip6gre_link_ops __read_mostly = {
-- 
2.8.0.rc2

^ permalink raw reply related

* [PATCH v2 next-next 07/12] fou: Add encap ops for IPv6 tunnels
From: Tom Herbert @ 2016-05-06 20:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1462565096-4076043-1-git-send-email-tom@herbertland.com>

Thsi packet adds IP tunnel encapsulation operations for IPv6. This
includes the infrastructure to add and delete operations. IPv6 variants
for fou6_build_header and gue6_build_header are added in a new
fou6 module. These encapsulation operations for fou and gue are
automatically added when the fou6 module loads.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/fou.h          |   2 +-
 include/net/ip6_tunnel.h   |  14 +++++
 net/ipv6/Makefile          |   4 +-
 net/ipv6/fou6.c            | 140 +++++++++++++++++++++++++++++++++++++++++++++
 net/ipv6/ip6_tunnel_core.c |  44 ++++++++++++++
 5 files changed, 202 insertions(+), 2 deletions(-)
 create mode 100644 net/ipv6/fou6.c
 create mode 100644 net/ipv6/ip6_tunnel_core.c

diff --git a/include/net/fou.h b/include/net/fou.h
index 7d2fda2..f5cc691 100644
--- a/include/net/fou.h
+++ b/include/net/fou.h
@@ -9,7 +9,7 @@
 #include <net/udp.h>
 
 size_t fou_encap_hlen(struct ip_tunnel_encap *e);
-static size_t gue_encap_hlen(struct ip_tunnel_encap *e);
+size_t gue_encap_hlen(struct ip_tunnel_encap *e);
 
 int __fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 		       u8 *protocol, __be16 *sport, int type);
diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index fb9e015..1c14c27 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -34,6 +34,20 @@ struct __ip6_tnl_parm {
 	__be32			o_key;
 };
 
+struct ip6_tnl_encap_ops {
+	size_t (*encap_hlen)(struct ip_tunnel_encap *e);
+	int (*build_header)(struct sk_buff *skb, struct ip_tunnel_encap *e,
+			    u8 *protocol, struct flowi6 *fl6);
+};
+
+extern const struct ip6_tnl_encap_ops __rcu *
+		ip6tun_encaps[MAX_IPTUN_ENCAP_OPS];
+
+int ip6_tnl_encap_add_ops(const struct ip6_tnl_encap_ops *op,
+			  unsigned int num);
+int ip6_tnl_encap_del_ops(const struct ip6_tnl_encap_ops *op,
+			  unsigned int num);
+
 /* IPv6 tunnel */
 struct ip6_tnl {
 	struct ip6_tnl __rcu *next;	/* next tunnel in list */
diff --git a/net/ipv6/Makefile b/net/ipv6/Makefile
index 5e9d6bf..5cf4a1f 100644
--- a/net/ipv6/Makefile
+++ b/net/ipv6/Makefile
@@ -9,7 +9,7 @@ ipv6-objs :=	af_inet6.o anycast.o ip6_output.o ip6_input.o addrconf.o \
 		route.o ip6_fib.o ipv6_sockglue.o ndisc.o udp.o udplite.o \
 		raw.o icmp.o mcast.o reassembly.o tcp_ipv6.o ping.o \
 		exthdrs.o datagram.o ip6_flowlabel.o inet6_connection_sock.o \
-		udp_offload.o
+		udp_offload.o ip6_tunnel_core.o
 
 ipv6-offload :=	ip6_offload.o tcpv6_offload.o exthdrs_offload.o
 
@@ -43,6 +43,8 @@ obj-$(CONFIG_IPV6_SIT) += sit.o
 obj-$(CONFIG_IPV6_TUNNEL) += ip6_tunnel.o
 obj-$(CONFIG_IPV6_GRE) += ip6_gre.o
 
+obj-$(CONFIG_NET_FOU) += fou6.o
+
 obj-y += addrconf_core.o exthdrs_core.o ip6_checksum.o ip6_icmp.o
 obj-$(CONFIG_INET) += output_core.o protocol.o $(ipv6-offload)
 
diff --git a/net/ipv6/fou6.c b/net/ipv6/fou6.c
new file mode 100644
index 0000000..c972d0b
--- /dev/null
+++ b/net/ipv6/fou6.c
@@ -0,0 +1,140 @@
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/socket.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+#include <linux/udp.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <net/fou.h>
+#include <net/ip.h>
+#include <net/ip6_tunnel.h>
+#include <net/ip6_checksum.h>
+#include <net/protocol.h>
+#include <net/udp.h>
+#include <net/udp_tunnel.h>
+
+static void fou6_build_udp(struct sk_buff *skb, struct ip_tunnel_encap *e,
+			   struct flowi6 *fl6, u8 *protocol, __be16 sport)
+{
+	struct udphdr *uh;
+
+	skb_push(skb, sizeof(struct udphdr));
+	skb_reset_transport_header(skb);
+
+	uh = udp_hdr(skb);
+
+	uh->dest = e->dport;
+	uh->source = sport;
+	uh->len = htons(skb->len);
+	udp6_set_csum(!(e->flags & TUNNEL_ENCAP_FLAG_CSUM6), skb,
+		      &fl6->saddr, &fl6->daddr, skb->len);
+
+	*protocol = IPPROTO_UDP;
+}
+
+int fou6_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
+		      u8 *protocol, struct flowi6 *fl6)
+{
+	__be16 sport;
+	int err;
+	int type = e->flags & TUNNEL_ENCAP_FLAG_CSUM6 ?
+		SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
+
+	err = __fou_build_header(skb, e, protocol, &sport, type);
+	if (err)
+		return err;
+
+	fou6_build_udp(skb, e, fl6, protocol, sport);
+
+	return 0;
+}
+EXPORT_SYMBOL(fou6_build_header);
+
+int gue6_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
+		      u8 *protocol, struct flowi6 *fl6)
+{
+	__be16 sport;
+	int err;
+	int type = e->flags & TUNNEL_ENCAP_FLAG_CSUM6 ?
+		SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
+
+	err = __gue_build_header(skb, e, protocol, &sport, type);
+	if (err)
+		return err;
+
+	fou6_build_udp(skb, e, fl6, protocol, sport);
+
+	return 0;
+}
+EXPORT_SYMBOL(gue6_build_header);
+
+#ifdef CONFIG_NET_FOU_IP_TUNNELS
+
+static const struct ip6_tnl_encap_ops fou_ip6tun_ops = {
+	.encap_hlen = fou_encap_hlen,
+	.build_header = fou6_build_header,
+};
+
+static const struct ip6_tnl_encap_ops gue_ip6tun_ops = {
+	.encap_hlen = gue_encap_hlen,
+	.build_header = gue6_build_header,
+};
+
+static int ip6_tnl_encap_add_fou_ops(void)
+{
+	int ret;
+
+	ret = ip6_tnl_encap_add_ops(&fou_ip6tun_ops, TUNNEL_ENCAP_FOU);
+	if (ret < 0) {
+		pr_err("can't add fou6 ops\n");
+		return ret;
+	}
+
+	ret = ip6_tnl_encap_add_ops(&gue_ip6tun_ops, TUNNEL_ENCAP_GUE);
+	if (ret < 0) {
+		pr_err("can't add gue6 ops\n");
+		ip6_tnl_encap_del_ops(&fou_ip6tun_ops, TUNNEL_ENCAP_FOU);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void ip6_tnl_encap_del_fou_ops(void)
+{
+	ip6_tnl_encap_del_ops(&fou_ip6tun_ops, TUNNEL_ENCAP_FOU);
+	ip6_tnl_encap_del_ops(&gue_ip6tun_ops, TUNNEL_ENCAP_GUE);
+}
+
+#else
+
+static int ip6_tnl_encap_add_fou_ops(void)
+{
+	return 0;
+}
+
+static void ip6_tnl_encap_del_fou_ops(void)
+{
+}
+
+#endif
+
+static int __init fou6_init(void)
+{
+	int ret;
+
+	ret = ip6_tnl_encap_add_fou_ops();
+
+	return ret;
+}
+
+static void __exit fou6_fini(void)
+{
+	ip6_tnl_encap_del_fou_ops();
+}
+
+module_init(fou6_init);
+module_exit(fou6_fini);
+MODULE_AUTHOR("Tom Herbert <therbert@google.com>");
+MODULE_LICENSE("GPL");
diff --git a/net/ipv6/ip6_tunnel_core.c b/net/ipv6/ip6_tunnel_core.c
new file mode 100644
index 0000000..5f5b79e
--- /dev/null
+++ b/net/ipv6/ip6_tunnel_core.c
@@ -0,0 +1,44 @@
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/if.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/net.h>
+#include <linux/in6.h>
+#include <net/ip6_tunnel.h>
+
+const struct ip6_tnl_encap_ops __rcu *
+		ip6tun_encaps[MAX_IPTUN_ENCAP_OPS] __read_mostly;
+EXPORT_SYMBOL(ip6tun_encaps);
+
+int ip6_tnl_encap_add_ops(const struct ip6_tnl_encap_ops *ops,
+			  unsigned int num)
+{
+	if (num >= MAX_IPTUN_ENCAP_OPS)
+		return -ERANGE;
+
+	return !cmpxchg((const struct ip6_tnl_encap_ops **)
+			&ip6tun_encaps[num],
+			NULL, ops) ? 0 : -1;
+}
+EXPORT_SYMBOL(ip6_tnl_encap_add_ops);
+
+int ip6_tnl_encap_del_ops(const struct ip6_tnl_encap_ops *ops,
+			  unsigned int num)
+{
+	int ret;
+
+	if (num >= MAX_IPTUN_ENCAP_OPS)
+		return -ERANGE;
+
+	ret = (cmpxchg((const struct ip6_tnl_encap_ops **)
+		       &ip6tun_encaps[num],
+		       ops, NULL) == ops) ? 0 : -1;
+
+	synchronize_net();
+
+	return ret;
+}
+EXPORT_SYMBOL(ip6_tnl_encap_del_ops);
+
-- 
2.8.0.rc2

^ permalink raw reply related

* [PATCH v2 next-next 09/12] ipv6: Change "final" protocol processing for encapsulation
From: Tom Herbert @ 2016-05-06 20:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1462565096-4076043-1-git-send-email-tom@herbertland.com>

When performing foo-over-UDP, UDP are receveived processed by the
encapsulation header which returns another protocol to process.
This may result in processing two (or more) protocols in the
loop that are marked as INET6_PROTO_FINAL. The actions taken
for hitting a final protocol, in particular the skb_postpull_rcsum
can only be performed.

This patch set adds a check of a final protocol has been seen. The
rules are:
  - If the final protocol has not been seen any protocol is processed
    (final and non-final). In the case of a final protocol, the final
    actions are taken (like the skb_postpull_rcsum)
  - If a final protocol has been seen (e.g. an encapsulating UDP
    header) then no further non-final protocols are allowed
    (e.g. extension headers). For more final protocols the
    final actions are not taken (e.g. skb_postpull_rcsum).

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/ipv6/ip6_input.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 2a0258a..7d98d01 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -216,6 +216,7 @@ static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *sk
 	unsigned int nhoff;
 	int nexthdr;
 	bool raw;
+	bool have_final = false;
 
 	/*
 	 *	Parse extension headers
@@ -235,9 +236,21 @@ resubmit:
 	if (ipprot) {
 		int ret;
 
-		if (ipprot->flags & INET6_PROTO_FINAL) {
+		if (have_final) {
+			if (!(ipprot->flags & INET6_PROTO_FINAL)) {
+				/* Once we've seen a final protocol don't
+				 * allow encapsulation on any non-final
+				 * ones. This allows foo in UDP encapsulation
+				 * to work.
+				 */
+				goto discard;
+			}
+		} else if (ipprot->flags & INET6_PROTO_FINAL) {
 			const struct ipv6hdr *hdr;
 
+			/* Only do this once for first final protocol */
+			have_final = true;
+
 			/* Free reference early: we don't need it any more,
 			   and it may hold ip_conntrack module loaded
 			   indefinitely. */
-- 
2.8.0.rc2

^ permalink raw reply related

* [PATCH v2 next-next 10/12] fou: Support IPv6 in fou
From: Tom Herbert @ 2016-05-06 20:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1462565096-4076043-1-git-send-email-tom@herbertland.com>

This patch adds receive path support for IPv6 with fou.

- Add address family to fou structure for open sockets. This supports
  AF_INET and AF_INET6. Lookups for fou ports are performed on both the
  port number and family.
- In fou and gue receive adjust tot_len in IPv4 header or payload_len
  based on address family.
- Allow AF_INET6 in FOU_ATTR_AF netlink attribute.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/ipv4/fou.c | 47 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 971c8c6..75db828 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -21,6 +21,7 @@ struct fou {
 	u8 protocol;
 	u8 flags;
 	__be16 port;
+	u8 family;
 	u16 type;
 	struct list_head list;
 	struct rcu_head rcu;
@@ -47,14 +48,17 @@ static inline struct fou *fou_from_sock(struct sock *sk)
 	return sk->sk_user_data;
 }
 
-static int fou_recv_pull(struct sk_buff *skb, size_t len)
+static int fou_recv_pull(struct sk_buff *skb, struct fou *fou, size_t len)
 {
-	struct iphdr *iph = ip_hdr(skb);
-
 	/* Remove 'len' bytes from the packet (UDP header and
 	 * FOU header if present).
 	 */
-	iph->tot_len = htons(ntohs(iph->tot_len) - len);
+	if (fou->family == AF_INET)
+		ip_hdr(skb)->tot_len = htons(ntohs(ip_hdr(skb)->tot_len) - len);
+	else
+		ipv6_hdr(skb)->payload_len =
+		    htons(ntohs(ipv6_hdr(skb)->payload_len) - len);
+
 	__skb_pull(skb, len);
 	skb_postpull_rcsum(skb, udp_hdr(skb), len);
 	skb_reset_transport_header(skb);
@@ -68,7 +72,7 @@ static int fou_udp_recv(struct sock *sk, struct sk_buff *skb)
 	if (!fou)
 		return 1;
 
-	if (fou_recv_pull(skb, sizeof(struct udphdr)))
+	if (fou_recv_pull(skb, fou, sizeof(struct udphdr)))
 		goto drop;
 
 	return -fou->protocol;
@@ -141,7 +145,11 @@ static int gue_udp_recv(struct sock *sk, struct sk_buff *skb)
 
 	hdrlen = sizeof(struct guehdr) + optlen;
 
-	ip_hdr(skb)->tot_len = htons(ntohs(ip_hdr(skb)->tot_len) - len);
+	if (fou->family == AF_INET)
+		ip_hdr(skb)->tot_len = htons(ntohs(ip_hdr(skb)->tot_len) - len);
+	else
+		ipv6_hdr(skb)->payload_len =
+		    htons(ntohs(ipv6_hdr(skb)->payload_len) - len);
 
 	/* Pull csum through the guehdr now . This can be used if
 	 * there is a remote checksum offload.
@@ -424,7 +432,8 @@ static int fou_add_to_port_list(struct net *net, struct fou *fou)
 
 	mutex_lock(&fn->fou_lock);
 	list_for_each_entry(fout, &fn->fou_list, list) {
-		if (fou->port == fout->port) {
+		if (fou->port == fout->port &&
+		    fou->family == fout->family) {
 			mutex_unlock(&fn->fou_lock);
 			return -EALREADY;
 		}
@@ -469,8 +478,9 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
 
 	sk = sock->sk;
 
-	fou->flags = cfg->flags;
 	fou->port = cfg->udp_config.local_udp_port;
+	fou->family = cfg->udp_config.family;
+	fou->flags = cfg->flags;
 	fou->type = cfg->type;
 	fou->sock = sock;
 
@@ -522,12 +532,13 @@ static int fou_destroy(struct net *net, struct fou_cfg *cfg)
 {
 	struct fou_net *fn = net_generic(net, fou_net_id);
 	__be16 port = cfg->udp_config.local_udp_port;
+	u8 family = cfg->udp_config.family;
 	int err = -EINVAL;
 	struct fou *fou;
 
 	mutex_lock(&fn->fou_lock);
 	list_for_each_entry(fou, &fn->fou_list, list) {
-		if (fou->port == port) {
+		if (fou->port == port && fou->family == family) {
 			fou_release(fou);
 			err = 0;
 			break;
@@ -565,8 +576,15 @@ static int parse_nl_config(struct genl_info *info,
 	if (info->attrs[FOU_ATTR_AF]) {
 		u8 family = nla_get_u8(info->attrs[FOU_ATTR_AF]);
 
-		if (family != AF_INET)
-			return -EINVAL;
+		switch (family) {
+		case AF_INET:
+			break;
+		case AF_INET6:
+			cfg->udp_config.ipv6_v6only = 1;
+			break;
+		default:
+			return -EAFNOSUPPORT;
+		}
 
 		cfg->udp_config.family = family;
 	}
@@ -657,6 +675,7 @@ static int fou_nl_cmd_get_port(struct sk_buff *skb, struct genl_info *info)
 	struct fou_cfg cfg;
 	struct fou *fout;
 	__be16 port;
+	u8 family;
 	int ret;
 
 	ret = parse_nl_config(info, &cfg);
@@ -666,6 +685,10 @@ static int fou_nl_cmd_get_port(struct sk_buff *skb, struct genl_info *info)
 	if (port == 0)
 		return -EINVAL;
 
+	family = cfg.udp_config.family;
+	if (family != AF_INET && family != AF_INET6)
+		return -EINVAL;
+
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
@@ -673,7 +696,7 @@ static int fou_nl_cmd_get_port(struct sk_buff *skb, struct genl_info *info)
 	ret = -ESRCH;
 	mutex_lock(&fn->fou_lock);
 	list_for_each_entry(fout, &fn->fou_list, list) {
-		if (port == fout->port) {
+		if (port == fout->port && family == fout->family) {
 			ret = fou_dump_info(fout, info->snd_portid,
 					    info->snd_seq, 0, msg,
 					    info->genlhdr->cmd);
-- 
2.8.0.rc2

^ permalink raw reply related

* [PATCH v2 next-next 01/12] gso: Remove arbitrary checks for unsupported GSO
From: Tom Herbert @ 2016-05-06 20:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: kernel-team
In-Reply-To: <1462565096-4076043-1-git-send-email-tom@herbertland.com>

In several gso_segment functions there are checks of gso_type against
a seemingly arbitrary list of SKB_GSO_* flags. This seems like an
attempt to identify unsupported GSO types, but since the stack is
the one that set these GSO types in the first place this seems
unnecessary to do. If a combination isn't valid in the first
place that stack should not allow setting it.

This is a code simplication especially for add new GSO types.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 net/ipv4/af_inet.c     | 18 ------------------
 net/ipv4/gre_offload.c | 14 --------------
 net/ipv4/tcp_offload.c | 19 -------------------
 net/ipv4/udp_offload.c | 10 ----------
 net/ipv6/ip6_offload.c | 18 ------------------
 net/ipv6/udp_offload.c | 13 -------------
 net/mpls/mpls_gso.c    |  9 ---------
 7 files changed, 101 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 2e6e65f..7f08d45 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1205,24 +1205,6 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
 	int ihl;
 	int id;
 
-	if (unlikely(skb_shinfo(skb)->gso_type &
-		     ~(SKB_GSO_TCPV4 |
-		       SKB_GSO_UDP |
-		       SKB_GSO_DODGY |
-		       SKB_GSO_TCP_ECN |
-		       SKB_GSO_GRE |
-		       SKB_GSO_GRE_CSUM |
-		       SKB_GSO_IPIP |
-		       SKB_GSO_SIT |
-		       SKB_GSO_TCPV6 |
-		       SKB_GSO_UDP_TUNNEL |
-		       SKB_GSO_UDP_TUNNEL_CSUM |
-		       SKB_GSO_TCP_FIXEDID |
-		       SKB_GSO_TUNNEL_REMCSUM |
-		       SKB_GSO_PARTIAL |
-		       0)))
-		goto out;
-
 	skb_reset_network_header(skb);
 	nhoff = skb_network_header(skb) - skb_mac_header(skb);
 	if (unlikely(!pskb_may_pull(skb, sizeof(*iph))))
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index e88190a..ecd1e09 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -26,20 +26,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
 	int gre_offset, outer_hlen;
 	bool need_csum, ufo;
 
-	if (unlikely(skb_shinfo(skb)->gso_type &
-				~(SKB_GSO_TCPV4 |
-				  SKB_GSO_TCPV6 |
-				  SKB_GSO_UDP |
-				  SKB_GSO_DODGY |
-				  SKB_GSO_TCP_ECN |
-				  SKB_GSO_TCP_FIXEDID |
-				  SKB_GSO_GRE |
-				  SKB_GSO_GRE_CSUM |
-				  SKB_GSO_IPIP |
-				  SKB_GSO_SIT |
-				  SKB_GSO_PARTIAL)))
-		goto out;
-
 	if (!skb->encapsulation)
 		goto out;
 
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 02737b6..5c59649 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -83,25 +83,6 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
 
 	if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
 		/* Packet is from an untrusted source, reset gso_segs. */
-		int type = skb_shinfo(skb)->gso_type;
-
-		if (unlikely(type &
-			     ~(SKB_GSO_TCPV4 |
-			       SKB_GSO_DODGY |
-			       SKB_GSO_TCP_ECN |
-			       SKB_GSO_TCP_FIXEDID |
-			       SKB_GSO_TCPV6 |
-			       SKB_GSO_GRE |
-			       SKB_GSO_GRE_CSUM |
-			       SKB_GSO_IPIP |
-			       SKB_GSO_SIT |
-			       SKB_GSO_UDP_TUNNEL |
-			       SKB_GSO_UDP_TUNNEL_CSUM |
-			       SKB_GSO_TUNNEL_REMCSUM |
-			       0) ||
-			     !(type & (SKB_GSO_TCPV4 |
-				       SKB_GSO_TCPV6))))
-			goto out;
 
 		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);
 
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 097060de..b556ef6 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -209,16 +209,6 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
 
 	if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
 		/* Packet is from an untrusted source, reset gso_segs. */
-		int type = skb_shinfo(skb)->gso_type;
-
-		if (unlikely(type & ~(SKB_GSO_UDP | SKB_GSO_DODGY |
-				      SKB_GSO_UDP_TUNNEL |
-				      SKB_GSO_UDP_TUNNEL_CSUM |
-				      SKB_GSO_TUNNEL_REMCSUM |
-				      SKB_GSO_IPIP |
-				      SKB_GSO_GRE | SKB_GSO_GRE_CSUM) ||
-			     !(type & (SKB_GSO_UDP))))
-			goto out;
 
 		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);
 
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index f5eb184..9ad743b 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -69,24 +69,6 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	bool encap, udpfrag;
 	int nhoff;
 
-	if (unlikely(skb_shinfo(skb)->gso_type &
-		     ~(SKB_GSO_TCPV4 |
-		       SKB_GSO_UDP |
-		       SKB_GSO_DODGY |
-		       SKB_GSO_TCP_ECN |
-		       SKB_GSO_TCP_FIXEDID |
-		       SKB_GSO_TCPV6 |
-		       SKB_GSO_GRE |
-		       SKB_GSO_GRE_CSUM |
-		       SKB_GSO_IPIP |
-		       SKB_GSO_SIT |
-		       SKB_GSO_UDP_TUNNEL |
-		       SKB_GSO_UDP_TUNNEL_CSUM |
-		       SKB_GSO_TUNNEL_REMCSUM |
-		       SKB_GSO_PARTIAL |
-		       0)))
-		goto out;
-
 	skb_reset_network_header(skb);
 	nhoff = skb_network_header(skb) - skb_mac_header(skb);
 	if (unlikely(!pskb_may_pull(skb, sizeof(*ipv6h))))
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 5429f6b..ac858c4 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -36,19 +36,6 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
 
 	if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
 		/* Packet is from an untrusted source, reset gso_segs. */
-		int type = skb_shinfo(skb)->gso_type;
-
-		if (unlikely(type & ~(SKB_GSO_UDP |
-				      SKB_GSO_DODGY |
-				      SKB_GSO_UDP_TUNNEL |
-				      SKB_GSO_UDP_TUNNEL_CSUM |
-				      SKB_GSO_TUNNEL_REMCSUM |
-				      SKB_GSO_GRE |
-				      SKB_GSO_GRE_CSUM |
-				      SKB_GSO_IPIP |
-				      SKB_GSO_SIT) ||
-			     !(type & (SKB_GSO_UDP))))
-			goto out;
 
 		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);
 
diff --git a/net/mpls/mpls_gso.c b/net/mpls/mpls_gso.c
index bbcf604..6de1e13 100644
--- a/net/mpls/mpls_gso.c
+++ b/net/mpls/mpls_gso.c
@@ -26,15 +26,6 @@ static struct sk_buff *mpls_gso_segment(struct sk_buff *skb,
 	netdev_features_t mpls_features;
 	__be16 mpls_protocol;
 
-	if (unlikely(skb_shinfo(skb)->gso_type &
-				~(SKB_GSO_TCPV4 |
-				  SKB_GSO_TCPV6 |
-				  SKB_GSO_UDP |
-				  SKB_GSO_DODGY |
-				  SKB_GSO_TCP_FIXEDID |
-				  SKB_GSO_TCP_ECN)))
-		goto out;
-
 	/* Setup inner SKB. */
 	mpls_protocol = skb->protocol;
 	skb->protocol = skb->inner_protocol;
-- 
2.8.0.rc2

^ permalink raw reply related


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