Netdev List
 help / color / mirror / Atom feed
* [net-next v2 13/16] i40e: increase ARQ size
From: Jeff Kirsher @ 2014-12-06 13:02 UTC (permalink / raw)
  To: davem; +Cc: Mitch Williams, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1417870933-17248-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Mitch Williams <mitch.a.williams@intel.com>

The ARQ needs to have at least as many entries as VFs, or the VFs will
get errors from the FW when they send messages to the PF. Since we don't
know how many VFs we'll end up with, just set up 128 descriptors.

Change-ID: I04ae3d1c7faf09110eb782214e9c05aeb62a6c59
Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 464342a..fc50f64 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -87,7 +87,7 @@
 #define I40E_MINIMUM_FCOE             1 /* minimum number of QPs for FCoE */
 #endif /* I40E_FCOE */
 #define I40E_MAX_AQ_BUF_SIZE          4096
-#define I40E_AQ_LEN                   32
+#define I40E_AQ_LEN                   128
 #define I40E_AQ_WORK_LIMIT            16
 #define I40E_MAX_USER_PRIORITY        8
 #define I40E_DEFAULT_MSG_ENABLE       4
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 12/16] i40e: Re enable Main VSI loopback setting in the reset path
From: Jeff Kirsher @ 2014-12-06 13:02 UTC (permalink / raw)
  To: davem
  Cc: Anjali Singhai Jain, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1417870933-17248-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Anjali Singhai Jain <anjali.singhai@intel.com>

There is an order in which this should happen. It turns out that FW will
not let you change the Loopback setting of the VSI with update VSI prior
to the VEB creation.

Change-ID: I7614ddff8b4c37702930c02f16f8c346aaa64bd1
Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 9aa2491..b38aa7a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5888,6 +5888,9 @@ static int i40e_reconstitute_veb(struct i40e_veb *veb)
 	if (ret)
 		goto end_reconstitute;
 
+	/* Enable LB mode for the main VSI now that it is on a VEB */
+	i40e_enable_pf_switch_lb(pf);
+
 	/* create the remaining VSIs attached to this VEB */
 	for (v = 0; v < pf->num_alloc_vsi; v++) {
 		if (!pf->vsi[v] || pf->vsi[v] == ctl_vsi)
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 15/16] i40e: Bump i40e version to 1.2.2 and i40evf version to 1.0.6
From: Jeff Kirsher @ 2014-12-06 13:02 UTC (permalink / raw)
  To: davem; +Cc: Catherine Sullivan, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1417870933-17248-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Catherine Sullivan <catherine.sullivan@intel.com>

Bump version.

Change-ID: I4264e81dcfb57ec46a3ede54b0a6cb25b497d3cb
Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c     | 4 ++--
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index b38aa7a..0a7ea4c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -38,8 +38,8 @@ static const char i40e_driver_string[] =
 #define DRV_KERN "-k"
 
 #define DRV_VERSION_MAJOR 1
-#define DRV_VERSION_MINOR 1
-#define DRV_VERSION_BUILD 23
+#define DRV_VERSION_MINOR 2
+#define DRV_VERSION_BUILD 2
 #define DRV_VERSION __stringify(DRV_VERSION_MAJOR) "." \
 	     __stringify(DRV_VERSION_MINOR) "." \
 	     __stringify(DRV_VERSION_BUILD)    DRV_KERN
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 62456f8..cabaf59 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -36,7 +36,7 @@ char i40evf_driver_name[] = "i40evf";
 static const char i40evf_driver_string[] =
 	"Intel(R) XL710/X710 Virtual Function Network Driver";
 
-#define DRV_VERSION "1.0.5"
+#define DRV_VERSION "1.0.6"
 const char i40evf_driver_version[] = DRV_VERSION;
 static const char i40evf_copyright[] =
 	"Copyright (c) 2013 - 2014 Intel Corporation.";
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 11/16] i40e: Add new update VSI flow to accommodate FW fix with VSI Loopback mode
From: Jeff Kirsher @ 2014-12-06 13:02 UTC (permalink / raw)
  To: davem
  Cc: Anjali Singhai Jain, netdev, nhorman, sassmann, jogreene,
	Jeff Kirsher
In-Reply-To: <1417870933-17248-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Anjali Singhai Jain <anjali.singhai@intel.com>

All VSIs on a VEB should either have loopback enabled or disabled, a
mixed mode is not supported for a VEB. Since our driver supports multiple
VSIs per PF that need to talk to each other make sure to enable Loopback
for the PF and FDIR VSI as well.

Also, we now have to explicitly enable Loopback mode otherwise we fail
VSI creation for VMDq and VF VSIs.

Change-ID: Ib68c3ea4aeb730ac9468f930610de456efbe5b20
Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c        | 14 +++++++++++++-
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |  4 +---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h |  1 +
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 2f4be25..9aa2491 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -7797,6 +7797,10 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)
 		ctxt.uplink_seid = vsi->uplink_seid;
 		ctxt.connection_type = 0x1;     /* regular data port */
 		ctxt.flags = I40E_AQ_VSI_TYPE_PF;
+		ctxt.info.valid_sections |=
+				cpu_to_le16(I40E_AQ_VSI_PROP_SWITCH_VALID);
+		ctxt.info.switch_id =
+				cpu_to_le16(I40E_AQ_VSI_SW_ID_FLAG_ALLOW_LB);
 		i40e_vsi_setup_queue_map(vsi, &ctxt, enabled_tc, true);
 		break;
 
@@ -8182,7 +8186,15 @@ struct i40e_vsi *i40e_vsi_setup(struct i40e_pf *pf, u8 type,
 		else if ((vsi->flags & I40E_VSI_FLAG_VEB_OWNER) == 0)
 			veb = i40e_veb_setup(pf, 0, vsi->uplink_seid, vsi->seid,
 					     vsi->tc_config.enabled_tc);
-
+		if (veb) {
+			if (vsi->seid != pf->vsi[pf->lan_vsi]->seid) {
+				dev_info(&vsi->back->pdev->dev,
+					 "%s: New VSI creation error, uplink seid of LAN VSI expected.\n",
+					 __func__);
+				return NULL;
+			}
+			i40e_enable_pf_switch_lb(pf);
+		}
 		for (i = 0; i < I40E_MAX_VEB && !veb; i++) {
 			if (pf->veb[i] && pf->veb[i]->seid == vsi->uplink_seid)
 				veb = pf->veb[i];
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 668d860..5bae895 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -707,7 +707,6 @@ complete_reset:
 	wr32(hw, I40E_VFGEN_RSTAT1(vf->vf_id), I40E_VFR_VFACTIVE);
 	i40e_flush(hw);
 }
-#ifdef CONFIG_PCI_IOV
 
 /**
  * i40e_enable_pf_switch_lb
@@ -715,7 +714,7 @@ complete_reset:
  *
  * enable switch loop back or die - no point in a return value
  **/
-static void i40e_enable_pf_switch_lb(struct i40e_pf *pf)
+void i40e_enable_pf_switch_lb(struct i40e_pf *pf)
 {
 	struct i40e_vsi *vsi = pf->vsi[pf->lan_vsi];
 	struct i40e_vsi_context ctxt;
@@ -742,7 +741,6 @@ static void i40e_enable_pf_switch_lb(struct i40e_pf *pf)
 			 __func__, vsi->back->hw.aq.asq_last_status);
 	}
 }
-#endif
 
 /**
  * i40e_disable_pf_switch_lb
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
index 0adc61e..9452f52 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h
@@ -126,5 +126,6 @@ int i40e_ndo_set_vf_spoofchk(struct net_device *netdev, int vf_id, bool enable);
 
 void i40e_vc_notify_link_state(struct i40e_pf *pf);
 void i40e_vc_notify_reset(struct i40e_pf *pf);
+void i40e_enable_pf_switch_lb(struct i40e_pf *pf);
 
 #endif /* _I40E_VIRTCHNL_PF_H_ */
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 14/16] i40e: get pf_id from HW rather than PCI function
From: Jeff Kirsher @ 2014-12-06 13:02 UTC (permalink / raw)
  To: davem; +Cc: Shannon Nelson, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1417870933-17248-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Shannon Nelson <shannon.nelson@intel.com>

Getting the pf_id from the function number was a good place to start,
but when the PF was setup in passthru mode, the PCI bus/device/function
was virtualized and the number in the VM is different from the number in
the bare metal.  This caused HW configuration issues when the wrong pf_id
was used to set up the HMC and other structures.  The PF_FUNC_RID register
has the real bus/device/function information as configured by the BIOS,
so use that for a better number.  This works in NPAR mode as well.

Change-ID: I65e3dd6c97594890c2bad566b83cc670b1dae534
Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
Acked-by: Greg Rose <gregory.v.rose@intel.com>
Acked-by: Kevin Scott <kevin.c.scott@intel.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_common.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index ec63bcb..3d741ee 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -550,7 +550,7 @@ struct i40e_rx_ptype_decoded i40e_ptype_lookup[] = {
 i40e_status i40e_init_shared_code(struct i40e_hw *hw)
 {
 	i40e_status status = 0;
-	u32 reg;
+	u32 port, ari, func_rid;
 
 	i40e_set_mac_type(hw);
 
@@ -563,18 +563,17 @@ i40e_status i40e_init_shared_code(struct i40e_hw *hw)
 
 	hw->phy.get_link_info = true;
 
-	/* Determine port number */
-	reg = rd32(hw, I40E_PFGEN_PORTNUM);
-	reg = ((reg & I40E_PFGEN_PORTNUM_PORT_NUM_MASK) >>
-	       I40E_PFGEN_PORTNUM_PORT_NUM_SHIFT);
-	hw->port = (u8)reg;
-
-	/* Determine the PF number based on the PCI fn */
-	reg = rd32(hw, I40E_GLPCI_CAPSUP);
-	if (reg & I40E_GLPCI_CAPSUP_ARI_EN_MASK)
-		hw->pf_id = (u8)((hw->bus.device << 3) | hw->bus.func);
+	/* Determine port number and PF number*/
+	port = (rd32(hw, I40E_PFGEN_PORTNUM) & I40E_PFGEN_PORTNUM_PORT_NUM_MASK)
+					   >> I40E_PFGEN_PORTNUM_PORT_NUM_SHIFT;
+	hw->port = (u8)port;
+	ari = (rd32(hw, I40E_GLPCI_CAPSUP) & I40E_GLPCI_CAPSUP_ARI_EN_MASK) >>
+						 I40E_GLPCI_CAPSUP_ARI_EN_SHIFT;
+	func_rid = rd32(hw, I40E_PF_FUNC_RID);
+	if (ari)
+		hw->pf_id = (u8)(func_rid & 0xff);
 	else
-		hw->pf_id = (u8)hw->bus.func;
+		hw->pf_id = (u8)(func_rid & 0x7);
 
 	status = i40e_init_nvm(hw);
 	return status;
-- 
1.9.3

^ permalink raw reply related

* [net-next v2 16/16] i40e: Reduce stack in i40e_dbg_dump_desc
From: Jeff Kirsher @ 2014-12-06 13:02 UTC (permalink / raw)
  To: davem; +Cc: Joe Perches, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1417870933-17248-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Joe Perches <joe@perches.com>

Reduce stack use by using kmemdup and not using a very
large struct on stack.

In function ‘i40e_dbg_dump_desc’:
warning: the frame size of 8192 bytes is larger than 2048 bytes [-Wframe-larger-than=]

Signed-off-by: Joe Perches <joe@perches.com>
Tested-by: Jim Young <jamesx.m.young@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 30 +++++++++++++++-----------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 16ac3f8..433a558 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -773,7 +773,7 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
 {
 	struct i40e_tx_desc *txd;
 	union i40e_rx_desc *rxd;
-	struct i40e_ring ring;
+	struct i40e_ring *ring;
 	struct i40e_vsi *vsi;
 	int i;
 
@@ -792,29 +792,32 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
 			 vsi_seid);
 		return;
 	}
-	if (is_rx_ring)
-		ring = *vsi->rx_rings[ring_id];
-	else
-		ring = *vsi->tx_rings[ring_id];
+
+	ring = kmemdup(is_rx_ring
+		       ? vsi->rx_rings[ring_id] : vsi->tx_rings[ring_id],
+		       sizeof(*ring), GFP_KERNEL);
+	if (!ring)
+		return;
+
 	if (cnt == 2) {
 		dev_info(&pf->pdev->dev, "vsi = %02i %s ring = %02i\n",
 			 vsi_seid, is_rx_ring ? "rx" : "tx", ring_id);
-		for (i = 0; i < ring.count; i++) {
+		for (i = 0; i < ring->count; i++) {
 			if (!is_rx_ring) {
-				txd = I40E_TX_DESC(&ring, i);
+				txd = I40E_TX_DESC(ring, i);
 				dev_info(&pf->pdev->dev,
 					 "   d[%03i] = 0x%016llx 0x%016llx\n",
 					 i, txd->buffer_addr,
 					 txd->cmd_type_offset_bsz);
 			} else if (sizeof(union i40e_rx_desc) ==
 				   sizeof(union i40e_16byte_rx_desc)) {
-				rxd = I40E_RX_DESC(&ring, i);
+				rxd = I40E_RX_DESC(ring, i);
 				dev_info(&pf->pdev->dev,
 					 "   d[%03i] = 0x%016llx 0x%016llx\n",
 					 i, rxd->read.pkt_addr,
 					 rxd->read.hdr_addr);
 			} else {
-				rxd = I40E_RX_DESC(&ring, i);
+				rxd = I40E_RX_DESC(ring, i);
 				dev_info(&pf->pdev->dev,
 					 "   d[%03i] = 0x%016llx 0x%016llx 0x%016llx 0x%016llx\n",
 					 i, rxd->read.pkt_addr,
@@ -823,26 +826,26 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
 			}
 		}
 	} else if (cnt == 3) {
-		if (desc_n >= ring.count || desc_n < 0) {
+		if (desc_n >= ring->count || desc_n < 0) {
 			dev_info(&pf->pdev->dev,
 				 "descriptor %d not found\n", desc_n);
 			return;
 		}
 		if (!is_rx_ring) {
-			txd = I40E_TX_DESC(&ring, desc_n);
+			txd = I40E_TX_DESC(ring, desc_n);
 			dev_info(&pf->pdev->dev,
 				 "vsi = %02i tx ring = %02i d[%03i] = 0x%016llx 0x%016llx\n",
 				 vsi_seid, ring_id, desc_n,
 				 txd->buffer_addr, txd->cmd_type_offset_bsz);
 		} else if (sizeof(union i40e_rx_desc) ==
 			   sizeof(union i40e_16byte_rx_desc)) {
-			rxd = I40E_RX_DESC(&ring, desc_n);
+			rxd = I40E_RX_DESC(ring, desc_n);
 			dev_info(&pf->pdev->dev,
 				 "vsi = %02i rx ring = %02i d[%03i] = 0x%016llx 0x%016llx\n",
 				 vsi_seid, ring_id, desc_n,
 				 rxd->read.pkt_addr, rxd->read.hdr_addr);
 		} else {
-			rxd = I40E_RX_DESC(&ring, desc_n);
+			rxd = I40E_RX_DESC(ring, desc_n);
 			dev_info(&pf->pdev->dev,
 				 "vsi = %02i rx ring = %02i d[%03i] = 0x%016llx 0x%016llx 0x%016llx 0x%016llx\n",
 				 vsi_seid, ring_id, desc_n,
@@ -852,6 +855,7 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
 	} else {
 		dev_info(&pf->pdev->dev, "dump desc rx/tx <vsi_seid> <ring_id> [<desc_n>]\n");
 	}
+	kfree(ring);
 }
 
 /**
-- 
1.9.3

^ permalink raw reply related

* Re: [net-next v2 07/16] i40e: Define and use i40e_is_vf macro
From: Joe Perches @ 2014-12-06 13:18 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Anjali Singhai Jain, netdev, nhorman, sassmann, jogreene
In-Reply-To: <1417870933-17248-8-git-send-email-jeffrey.t.kirsher@intel.com>

On Sat, 2014-12-06 at 05:02 -0800, Jeff Kirsher wrote:
> From: Anjali Singhai Jain <anjali.singhai@intel.com>
> 
> This patch is useful for future expansion when new VF MAC types get
> added. It helps with cleaning up VF driver flow.
[]
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
[]
> +#define i40e_is_vf(_hw)	((_hw)->mac.type == I40E_MAC_VF)
[]
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_type.h b/drivers/net/ethernet/intel/i40evf/i40e_type.h
[]
> +#define i40e_is_vf(_hw)	((_hw)->mac.type == I40E_MAC_VF)

These are probably better as static inline

^ permalink raw reply

* [patch v2] ipvs: uninitialized data with IP_VS_IPV6
From: Dan Carpenter @ 2014-12-06 13:49 UTC (permalink / raw)
  To: Wensong Zhang
  Cc: Simon Horman, Julian Anastasov, Pablo Neira Ayuso,
	Patrick McHardy, Jozsef Kadlecsik, David S. Miller, netdev,
	lvs-devel, netfilter-devel, coreteam, kernel-janitors
In-Reply-To: <alpine.LFD.2.11.1412042308100.4841@ja.home.ssi.bg>

The app_tcp_pkt_out() function expects "*diff" to be set and ends up
using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.

The same issue is there in app_tcp_pkt_in().  Thanks to Julian Anastasov
for noticing that.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: fix app_tcp_pkt_in() as well.  This is an old bug.

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 1d5341f..5d3daae 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -183,6 +183,8 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 	struct nf_conn *ct;
 	struct net *net;
 
+	*diff = 0;
+
 #ifdef CONFIG_IP_VS_IPV6
 	/* This application helper doesn't work with IPv6 yet,
 	 * so turn this into a no-op for IPv6 packets
@@ -191,8 +193,6 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		return 1;
 #endif
 
-	*diff = 0;
-
 	/* Only useful for established sessions */
 	if (cp->state != IP_VS_TCP_S_ESTABLISHED)
 		return 1;
@@ -322,6 +322,9 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
 	struct ip_vs_conn *n_cp;
 	struct net *net;
 
+	/* no diff required for incoming packets */
+	*diff = 0;
+
 #ifdef CONFIG_IP_VS_IPV6
 	/* This application helper doesn't work with IPv6 yet,
 	 * so turn this into a no-op for IPv6 packets
@@ -330,9 +333,6 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		return 1;
 #endif
 
-	/* no diff required for incoming packets */
-	*diff = 0;

^ permalink raw reply related

* [PATCH] i40e: Fix possible memory leak in i40e_dbg_dump_desc
From: Joe Perches @ 2014-12-06 14:03 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: kbuild, Shannon Nelson, Dan Carpenter, netdev, nhorman, sassmann,
	jogreene
In-Reply-To: <20141206134513.GD4963@mwanda>

I didn't notice that return in the code, fix it by
adding a goto out instead to free the memory.

Fixes:

> New smatch warnings:
> drivers/net/ethernet/intel/i40e/i40e_debugfs.c:832 i40e_dbg_dump_desc() warn: possible memory leak of 'ring'

Signed-off-by: Joe Perches <joe@perches.com>
Noticed-by: Dan Carpenter <dan.carpenter@oracle.com> (go smatch)
---

Jeff Kirsher can legitimately give me a hard time now...
Thanks Dan.

 drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 433a558..cb0de45 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -829,7 +829,7 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
 		if (desc_n >= ring->count || desc_n < 0) {
 			dev_info(&pf->pdev->dev,
 				 "descriptor %d not found\n", desc_n);
-			return;
+			goto out;
 		}
 		if (!is_rx_ring) {
 			txd = I40E_TX_DESC(ring, desc_n);
@@ -855,6 +855,8 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
 	} else {
 		dev_info(&pf->pdev->dev, "dump desc rx/tx <vsi_seid> <ring_id> [<desc_n>]\n");
 	}
+
+out:
 	kfree(ring);
 }
 

^ permalink raw reply related

* [PATCH 3.12 60/66] net/ping: handle protocol mismatching scenario
From: Jiri Slaby @ 2014-12-06 15:07 UTC (permalink / raw)
  To: stable
  Cc: linux-kernel, Jane Zhou, David S. Miller, Alexey Kuznetsov,
	James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev,
	Yiwei Zhao, Jiri Slaby
In-Reply-To: <cover.1417878427.git.jslaby@suse.cz>

From: Jane Zhou <a17711@motorola.com>

3.12-stable review patch.  If anyone has any objections, please let me know.

===============

commit 91a0b603469069cdcce4d572b7525ffc9fd352a6 upstream.

ping_lookup() may return a wrong sock if sk_buff's and sock's protocols
dont' match. For example, sk_buff's protocol is ETH_P_IPV6, but sock's
sk_family is AF_INET, in that case, if sk->sk_bound_dev_if is zero, a wrong
sock will be returned.
the fix is to "continue" the searching, if no matching, return NULL.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Jane Zhou <a17711@motorola.com>
Signed-off-by: Yiwei Zhao <gbjc64@motorola.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 net/ipv4/ping.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index c211607b79b3..8bd51f49aa96 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -214,6 +214,8 @@ static struct sock *ping_lookup(struct net *net, struct sk_buff *skb, u16 ident)
 					     &ipv6_hdr(skb)->daddr))
 				continue;
 #endif
+		} else {
+			continue;
 		}
 
 		if (sk->sk_bound_dev_if && sk->sk_bound_dev_if != dif)
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH v2 net-next] tcp: refine TSO autosizing
From: Eric Dumazet @ 2014-12-06 16:58 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Nandita Dukkipati
In-Reply-To: <1417832560.15618.29.camel@edumazet-glaptop2.roam.corp.google.com>

On Fri, 2014-12-05 at 18:22 -0800, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 

...

> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> ---
> v2: added tcp_tso_autosize() helper and removed tp->xmit_size_goal_segs

Please do not apply this patch. I'll send a v3.

We still need to cook skb with a multiple of mss bytes.

^ permalink raw reply

* Re: AW: ixgbe: Regression, unsupported SFP+ modules on 10Gbit/s X520 NIC no longer work with allow_unsupported_sfp=1
From: Martin Bosner @ 2014-12-06 17:08 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1361367596.7173.52.camel@jtkirshe-mobl>

Jeff Kirsher <jeffrey.t.kirsher <at> intel.com> writes:

> 
> We are currently working on the resolution to your issue.  I have also
> added your patch to my queue of ixgbe patches.
> 
> Cheers,
> Jeff
> 

Any news on this?

^ permalink raw reply

* Re: [PATCH v3 iproute2] bridge link: add option 'self'
From: Scott Feldman @ 2014-12-06 17:12 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: Jiří Pírko, Jamal Hadi Salim, Benjamin LaHaise,
	Thomas Graf, john fastabend, stephen@networkplumber.org,
	John Linville, vyasevic@redhat.com, Netdev, David S. Miller,
	shm@cumulusnetworks.com, Andy Gospodarek
In-Reply-To: <1417854061-4675-1-git-send-email-roopa@cumulusnetworks.com>

Signed-of-by: Scott Feldman <sfeldma@gmail.com>

On Sat, Dec 6, 2014 at 12:21 AM,  <roopa@cumulusnetworks.com> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Currently self is set internally only if hwmode is set.
> This makes it necessary for the hw to have a mode.
> There is no hwmode really required to go to hardware. So, introduce
> self for anybody who wants to target hardware.
>
> v1 -> v2
>     - fix a few bugs. Initialize flags to zero: this was required to
>     keep the current behaviour unchanged.
>
> v2 -> v3
>     - fix comment
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> ---
>  bridge/link.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/bridge/link.c b/bridge/link.c
> index 90d9e7f..3f77aab 100644
> --- a/bridge/link.c
> +++ b/bridge/link.c
> @@ -261,7 +261,7 @@ static int brlink_modify(int argc, char **argv)
>         __s16 priority = -1;
>         __s8 state = -1;
>         __s16 mode = -1;
> -       __u16 flags = BRIDGE_FLAGS_MASTER;
> +       __u16 flags = 0;
>         struct rtattr *nest;
>
>         memset(&req, 0, sizeof(req));
> @@ -321,6 +321,8 @@ static int brlink_modify(int argc, char **argv)
>                                         "\"veb\".\n");
>                                 exit(-1);
>                         }
> +               } else if (strcmp(*argv, "self") == 0) {
> +                       flags = BRIDGE_FLAGS_SELF;
>                 } else {
>                         usage();
>                 }
> @@ -369,16 +371,16 @@ static int brlink_modify(int argc, char **argv)
>
>         addattr_nest_end(&req.n, nest);
>
> -       /* IFLA_AF_SPEC nested attribute.  Contains IFLA_BRIDGE_FLAGS that
> -        * designates master or self operation as well as 'vepa' or 'veb'
> -        * operation modes.  These are only valid in 'self' mode on some
> -        * devices so far.  Thus we only need to include the flags attribute
> -        * if we are setting the hw mode.
> +       /* IFLA_AF_SPEC nested attribute. Contains IFLA_BRIDGE_FLAGS that
> +        * designates master or self operation and IFLA_BRIDGE_MODE
> +        * for hw 'vepa' or 'veb' operation modes. The hwmodes are
> +        * only valid in 'self' mode on some devices so far.
>          */
> -       if (mode >= 0) {
> +       if (mode >= 0 || flags > 0) {
>                 nest = addattr_nest(&req.n, sizeof(req), IFLA_AF_SPEC);
>
> -               addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags);
> +               if (flags > 0)
> +                       addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags);
>
>                 if (mode >= 0)
>                         addattr16(&req.n, sizeof(req), IFLA_BRIDGE_MODE, mode);
> --
> 1.7.10.4
>

^ permalink raw reply

* [PATCH net-next] dst: no need to take reference on DST_NOCACHE dsts
From: Hannes Frederic Sowa @ 2014-12-06 18:19 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, Julian Anastasov

Since commit f8864972126899 ("ipv4: fix dst race in sk_dst_get()")
DST_NOCACHE dst_entries get freed by RCU. So there is no need to get a
reference on them when we are in rcu protected sections.

Cc: Eric Dumazet <edumazet@google.com>
Cc: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/linux/skbuff.h          | 23 ++---------------------
 net/core/dst.c                  | 24 ------------------------
 net/netfilter/ipvs/ip_vs_xmit.c |  4 ++--
 3 files changed, 4 insertions(+), 47 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7691ad5..70ebc79 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -718,9 +718,6 @@ static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
 	skb->_skb_refdst = (unsigned long)dst;
 }
 
-void __skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst,
-			 bool force);
-
 /**
  * skb_dst_set_noref - sets skb dst, hopefully, without taking reference
  * @skb: buffer
@@ -733,24 +730,8 @@ void __skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst,
  */
 static inline void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
 {
-	__skb_dst_set_noref(skb, dst, false);
-}
-
-/**
- * skb_dst_set_noref_force - sets skb dst, without taking reference
- * @skb: buffer
- * @dst: dst entry
- *
- * Sets skb dst, assuming a reference was not taken on dst.
- * No reference is taken and no dst_release will be called. While for
- * cached dsts deferred reclaim is a basic feature, for entries that are
- * not cached it is caller's job to guarantee that last dst_release for
- * provided dst happens when nobody uses it, eg. after a RCU grace period.
- */
-static inline void skb_dst_set_noref_force(struct sk_buff *skb,
-					   struct dst_entry *dst)
-{
-	__skb_dst_set_noref(skb, dst, true);
+	WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
+	skb->_skb_refdst = (unsigned long)dst | SKB_DST_NOREF;
 }
 
 /**
diff --git a/net/core/dst.c b/net/core/dst.c
index a028409..e956ce6 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -327,30 +327,6 @@ void __dst_destroy_metrics_generic(struct dst_entry *dst, unsigned long old)
 }
 EXPORT_SYMBOL(__dst_destroy_metrics_generic);
 
-/**
- * __skb_dst_set_noref - sets skb dst, without a reference
- * @skb: buffer
- * @dst: dst entry
- * @force: if force is set, use noref version even for DST_NOCACHE entries
- *
- * Sets skb dst, assuming a reference was not taken on dst
- * skb_dst_drop() should not dst_release() this dst
- */
-void __skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst, bool force)
-{
-	WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
-	/* If dst not in cache, we must take a reference, because
-	 * dst_release() will destroy dst as soon as its refcount becomes zero
-	 */
-	if (unlikely((dst->flags & DST_NOCACHE) && !force)) {
-		dst_hold(dst);
-		skb_dst_set(skb, dst);
-	} else {
-		skb->_skb_refdst = (unsigned long)dst | SKB_DST_NOREF;
-	}
-}
-EXPORT_SYMBOL(__skb_dst_set_noref);
-
 /* Dirty hack. We did it in 2.2 (in __dst_free),
  * we have _very_ good reasons not to repeat
  * this mistake in 2.3, but we have no choice
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 1f93313..3aedbda 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -343,7 +343,7 @@ __ip_vs_get_out_rt(int skb_af, struct sk_buff *skb, struct ip_vs_dest *dest,
 	skb_dst_drop(skb);
 	if (noref) {
 		if (!local)
-			skb_dst_set_noref_force(skb, &rt->dst);
+			skb_dst_set_noref(skb, &rt->dst);
 		else
 			skb_dst_set(skb, dst_clone(&rt->dst));
 	} else
@@ -487,7 +487,7 @@ __ip_vs_get_out_rt_v6(int skb_af, struct sk_buff *skb, struct ip_vs_dest *dest,
 	skb_dst_drop(skb);
 	if (noref) {
 		if (!local)
-			skb_dst_set_noref_force(skb, &rt->dst);
+			skb_dst_set_noref(skb, &rt->dst);
 		else
 			skb_dst_set(skb, dst_clone(&rt->dst));
 	} else
-- 
1.9.3

^ permalink raw reply related

* Vorgeschlagene Finanzdarlehen...
From: nicole dulac @ 2014-12-06 18:50 UTC (permalink / raw)


Vorgeschlagene Finanzdarlehen
Ich bin ein und bietet privater Investor Raha Darlehen für alle
Personen, zu haben und gute Manieren in der Lage, mit einem Zinssatz
von und 3% einer von Laufzeit 1-30 aastat zurück, von der abhängig
gewünschten Menge.
Ich will meine Brüder und einfach nur Schwestern surevad finanziellen
Schwierigkeiten sind, und sind von den Banken Daher für weitere
Informationen zu helfen abgelehnt, kontaktieren saatke mulle bitte kohta
E-Mail: nicoledulac01@gmail.com
PS: Ich werde auch zu geben Meinen Vollen Namen, ist nichts verborgen,
Weil ich mit Vertrauen und Ehrlichkeit. In Wartestellung ...
nicoledulac01@gmail.com
nicoledulac01@gmail.com

^ permalink raw reply

* Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads
From: Roopa Prabhu @ 2014-12-06 18:59 UTC (permalink / raw)
  To: Thomas Graf
  Cc: jiri, sfeldma, jhs, bcrl, john.fastabend, stephen, linville,
	nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh, aviadr,
	netdev, davem, shm, gospo
In-Reply-To: <20141206101425.GA15999@casper.infradead.org>

On 12/6/14, 2:14 AM, Thomas Graf wrote:
> On 12/05/14 at 11:46pm, Roopa Prabhu wrote:
>> On 12/5/14, 2:43 PM, Thomas Graf wrote:
>>> On 12/04/14 at 06:26pm, roopa@cumulusnetworks.com wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This is a generic high level feature flag for all switch asic features today.
>>>>
>>>> switch drivers set this flag on switch ports. Logical devices like
>>>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>>>
>>>> I had to use SWITCH in the name to avoid ambiguity with other feature
>>>> flags. But, since i have been harping about not calling it 'switch',
>>>> I am welcome to any suggestions :)
>>>>
>>>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>>>> in net_device_flags.
>>> What does this flag indicate specifically? What driver would
>>> implement ndo_bridge_setlink() but not set this flag?
>>>
>>> I think it should be clearly documented when this flag is to bet set.
>> I mentioned it as an alternative because it was there in my RFC patch. There
>> is no code for it yet.
>> And I will get rid of the comment in v2.
> Sorry, I was referring to NETIF_F_HW_SWITCH_OFFLOAD.
Ah, ok.

> I do not
> understand the scope of this bit/flag yet.
> Can you give examples
> when to set this bit? At what point would the existing ixgbe FDB
> offload set this bit? Is there a case when ndo_bridge_setlink()
> is implemented but this bit is not set?
The switch asic drivers will set this flag on the switch ports. This 
flag will be used when you want to offload
kernel networking to hardware or in other words accelerate  the 
corresponding kernel network function.
  In the context of current patches it is the bridge driver. A kernel 
bridge that has any of these switch asic ports
  as bridge ports can inherit this flag and use this flag on the ports to
call the switch driver ndo ops to offload state to hw and thus hw 
accelerate the bridging function.

The existing l2 offload flags, example the hwmode and self flags used 
for nic drivers bridge/l2 offload,
does not help switch asic hardware completely. AFAICT, those are used 
today to manage the bridge in nic hw directly.
They don't involve the bridge driver.

And the most important thing is they require the user to specify this 
additional flag to offload.


In my proposal,
- The flag/bit helps switch asic drivers to indicate that they 
accelerate the kernel networking objects/functions
- The user does not have to specify a new flag to do so. A bridge 
created with switch asic ports will thus be accelerated if the switch 
driver supports it.
- The user can continue to directly manage l2 in nics (ixgbe) using the 
existing hwmode/self flags
- And it also does not stop users from using the 'self' flag to talk to 
the switch asic driver directly
- And involving the bridge driver makes sure the add/del notifications 
to user space go out after both kernel and hardware are programmed

Regarding ixgbe driver setting this flag: For the current vepa and veb 
modes I don't see a need for it.
But if there is a use case in the future to indicate hw accelerating the 
bridge driver, ixgbe driver can set this flag

^ permalink raw reply

* Re: [net-next 04/14] ixgbe: remove CIAA/D register reads from bad VF check
From: Alex Williamson @ 2014-12-06 19:42 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: David Miller, emil.s.tantilov, netdev, nhorman, sassmann,
	jogreene
In-Reply-To: <1417841892.2399.2.camel@jtkirshe-mobl>

On Fri, 2014-12-05 at 20:58 -0800, Jeff Kirsher wrote:
> On Fri, 2014-12-05 at 20:49 -0800, David Miller wrote:
> > From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Date: Fri,  5 Dec 2014 09:52:43 -0800
> > 
> > > From: Emil Tantilov <emil.s.tantilov@intel.com>
> > > 
> > > Accessing the CIAA/D register can block access to the PCI config space.
> > > 
> > > This patch removes the read/write operations to the CIAA/D registers
> > > and makes use of standard kernel functions for accessing the PCI config
> > > space.
> > > 
> > > In addition it moves ixgbevf_check_for_bad_vf() into the watchdog subtask
> > > which reduces the frequency of the checks.
> > > 
> > > CC: Alex Williamson <alex.williamson@redhat.com>
> > > Reported-by: Alex Williamson <alex.williamson@redhat.com>
> > > Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
> > > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > 
> > Alex Willaimson stated that he'd like to see this for -stable, but I'm warning
> > right now that a change not appropriate for 'net' is not approperiate for
> > '-stable' either.
> 
> Agreed, only reason I did not send this to net (along with the other
> fixes by Emil) was that we are at -rc7 and do not consider these
> "critical" to try and squeeze in before the release.

I'm not trying to subvert any process, just trying to note that the
commit log is rather subtle for the severity of the bug this fixes and
this should be considered for stable once accepted.  Stable obviously
doesn't take anything that's not already upstream.  Thanks,

Alex

^ permalink raw reply

* Re: [ovs-discuss] kernel panic receiving flooded VXLAN traffic with OVS
From: Nicholas Bastin @ 2014-12-06 22:47 UTC (permalink / raw)
  To: Jesse Gross; +Cc: discuss-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev
In-Reply-To: <CAEP_g=_mfSCH1250ezx_h8_yM_4FzsYcsS6EnGi99AFoWO_MKw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1160 bytes --]

On Fri, Dec 5, 2014 at 4:51 PM, Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org> wrote:

> I don't think there is anything inherently wrong with aggregating TCP
> segments in VXLAN that are not destined for the local host. This is
> conceptually the same as doing aggregation for TCP packets where we
> only perform L2 bridging - in theory we shouldn't look at the upper
> layers but it is fine as long as we faithfully reconstruct it on the
> way out.
>

But you don't faithfully reconstruct what the user originally sent -
in-path reassembly is always wrong, which is why hardware switches don't do
it (by default, anyhow).  If you configure a middlebox to do some kind of
assembly/translation/whatever work for you, that's fine, but something that
advertises itself as a "switch" or "router" should definitely not do this
by default.

If you reassemble frames you completely obviate any kind of PMTU-D or
configured MTU that your user is using, and this breaks a lot of paths.  We
completely disable all GRO/TSO/etc., but if you are able to determine that
a packet is not destined for the local host you should definitely not
mutate it.

--
Nick

[-- Attachment #1.2: Type: text/html, Size: 1701 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
discuss mailing list
discuss@openvswitch.org
http://openvswitch.org/mailman/listinfo/discuss

^ permalink raw reply

* [PATCH net-next v2 0/4] net: allow setting congctl via routing table
From: Daniel Borkmann @ 2014-12-06 23:39 UTC (permalink / raw)
  To: davem; +Cc: hannes, fw, netdev

This is the second part of our work and allows for setting the congestion
control algorithm via routing table. For details, please see individual
patches.

Joint work with Florian Westphal, suggested by Hannes Frederic Sowa.

Patch for iproute2: http://patchwork.ozlabs.org/patch/418149/

Thanks!

v1->v2:
 - Very sorry, I noticed I had decnet disabled during testing.
   Added missing header include in decnet, rest as is.

Daniel Borkmann (4):
  net: tcp: refactor reinitialization of congestion control
  net: tcp: add key management to congestion control
  net: tcp: add RTAX_CC_ALGO fib handling
  net: tcp: add per route congestion control

 include/net/inet_connection_sock.h |  3 +-
 include/net/tcp.h                  | 22 +++++++++-
 include/uapi/linux/rtnetlink.h     |  2 +
 net/core/rtnetlink.c               | 15 ++++++-
 net/decnet/dn_fib.c                |  3 +-
 net/decnet/dn_table.c              |  4 +-
 net/ipv4/fib_semantics.c           | 14 +++++-
 net/ipv4/tcp_cong.c                | 87 ++++++++++++++++++++++++++++++++------
 net/ipv4/tcp_ipv4.c                |  2 +
 net/ipv4/tcp_minisocks.c           | 30 +++++++++++--
 net/ipv4/tcp_output.c              | 21 +++++++++
 net/ipv6/ip6_fib.c                 | 15 ++++++-
 net/ipv6/route.c                   |  3 +-
 net/ipv6/tcp_ipv6.c                |  2 +
 14 files changed, 197 insertions(+), 26 deletions(-)

-- 
1.7.11.7

^ permalink raw reply

* [PATCH net-next v2 1/4] net: tcp: refactor reinitialization of congestion control
From: Daniel Borkmann @ 2014-12-06 23:39 UTC (permalink / raw)
  To: davem; +Cc: hannes, fw, netdev
In-Reply-To: <1417909163-19135-1-git-send-email-dborkman@redhat.com>

We can just move this to an extra function and make the code
a bit more readable, no functional change.

Joint work with Florian Westphal.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/ipv4/tcp_cong.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 27ead0d..38f2f8a 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -107,6 +107,18 @@ void tcp_init_congestion_control(struct sock *sk)
 		icsk->icsk_ca_ops->init(sk);
 }
 
+static void tcp_reinit_congestion_control(struct sock *sk,
+					  const struct tcp_congestion_ops *ca)
+{
+	struct inet_connection_sock *icsk = inet_csk(sk);
+
+	tcp_cleanup_congestion_control(sk);
+	icsk->icsk_ca_ops = ca;
+
+	if (sk->sk_state != TCP_CLOSE && icsk->icsk_ca_ops->init)
+		icsk->icsk_ca_ops->init(sk);
+}
+
 /* Manage refcounts on socket close. */
 void tcp_cleanup_congestion_control(struct sock *sk)
 {
@@ -262,21 +274,13 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
 #endif
 	if (!ca)
 		err = -ENOENT;
-
 	else if (!((ca->flags & TCP_CONG_NON_RESTRICTED) ||
 		   ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)))
 		err = -EPERM;
-
 	else if (!try_module_get(ca->owner))
 		err = -EBUSY;
-
-	else {
-		tcp_cleanup_congestion_control(sk);
-		icsk->icsk_ca_ops = ca;
-
-		if (sk->sk_state != TCP_CLOSE && icsk->icsk_ca_ops->init)
-			icsk->icsk_ca_ops->init(sk);
-	}
+	else
+		tcp_reinit_congestion_control(sk, ca);
  out:
 	rcu_read_unlock();
 	return err;
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next v2 3/4] net: tcp: add RTAX_CC_ALGO fib handling
From: Daniel Borkmann @ 2014-12-06 23:39 UTC (permalink / raw)
  To: davem; +Cc: hannes, fw, netdev
In-Reply-To: <1417909163-19135-1-git-send-email-dborkman@redhat.com>

This patch adds the minimum necessary for the RTAX_CC_ALGO congestion
control metric to be set up and dumped back to user space.

While the internal representation of RTAX_CC_ALGO is handled as a u32
key, we avoided to expose this implementation detail to user space, thus
instead, we chose the netlink attribute that is being exchanged between
user space to be the actual congestion control algorithm name, similarly
as in the setsockopt(2) API in order to allow for maximum flexibility,
even for 3rd party modules.

It is a bit unfortunate that RTAX_QUICKACK used up a whole RTAX slot as
it should have been stored in RTAX_FEATURES instead, we first thought
about reusing it for the congestion control key, but it brings more
complications and/or confusion than worth it. Trying to load a non
present congestion algorithm name will be rejected.

Joint work with Florian Westphal.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 include/net/tcp.h              |  7 +++++++
 include/uapi/linux/rtnetlink.h |  2 ++
 net/core/rtnetlink.c           | 15 +++++++++++++--
 net/decnet/dn_fib.c            |  3 ++-
 net/decnet/dn_table.c          |  4 +++-
 net/ipv4/fib_semantics.c       | 14 ++++++++++++--
 net/ipv6/ip6_fib.c             | 15 ++++++++++++++-
 net/ipv6/route.c               |  3 ++-
 8 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 135b70c..95bb237 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -846,7 +846,14 @@ extern struct tcp_congestion_ops tcp_reno;
 
 struct tcp_congestion_ops *tcp_ca_find_key(u32 key);
 u32 tcp_ca_get_key_by_name(const char *name);
+#ifdef CONFIG_INET
 char *tcp_ca_get_name_by_key(u32 key, char *buffer);
+#else
+static inline char *tcp_ca_get_name_by_key(u32 key, char *buffer)
+{
+	return NULL;
+}
+#endif
 
 static inline bool tcp_ca_needs_ecn(const struct sock *sk)
 {
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 9c9b8b4..d81f22d 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -389,6 +389,8 @@ enum {
 #define RTAX_INITRWND RTAX_INITRWND
 	RTAX_QUICKACK,
 #define RTAX_QUICKACK RTAX_QUICKACK
+	RTAX_CC_ALGO,
+#define RTAX_CC_ALGO RTAX_CC_ALGO
 	__RTAX_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 61cb7e7..3566f41 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -50,6 +50,7 @@
 #include <net/arp.h>
 #include <net/route.h>
 #include <net/udp.h>
+#include <net/tcp.h>
 #include <net/sock.h>
 #include <net/pkt_sched.h>
 #include <net/fib_rules.h>
@@ -669,9 +670,19 @@ int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics)
 
 	for (i = 0; i < RTAX_MAX; i++) {
 		if (metrics[i]) {
+			if (i == RTAX_CC_ALGO - 1) {
+				char tmp[TCP_CA_NAME_MAX], *name;
+
+				name = tcp_ca_get_name_by_key(metrics[i], tmp);
+				if (!name)
+					continue;
+				if (nla_put_string(skb, i + 1, name))
+					goto nla_put_failure;
+			} else {
+				if (nla_put_u32(skb, i + 1, metrics[i]))
+					goto nla_put_failure;
+			}
 			valid++;
-			if (nla_put_u32(skb, i+1, metrics[i]))
-				goto nla_put_failure;
 		}
 	}
 
diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c
index d332aef..df48034 100644
--- a/net/decnet/dn_fib.c
+++ b/net/decnet/dn_fib.c
@@ -298,7 +298,8 @@ struct dn_fib_info *dn_fib_create_info(const struct rtmsg *r, struct nlattr *att
 			int type = nla_type(attr);
 
 			if (type) {
-				if (type > RTAX_MAX || nla_len(attr) < 4)
+				if (type > RTAX_MAX || type == RTAX_CC_ALGO ||
+				    nla_len(attr) < 4)
 					goto err_inval;
 
 				fi->fib_metrics[type-1] = nla_get_u32(attr);
diff --git a/net/decnet/dn_table.c b/net/decnet/dn_table.c
index 86e3807..3f19fcb 100644
--- a/net/decnet/dn_table.c
+++ b/net/decnet/dn_table.c
@@ -29,6 +29,7 @@
 #include <linux/route.h> /* RTF_xxx */
 #include <net/neighbour.h>
 #include <net/netlink.h>
+#include <net/tcp.h>
 #include <net/dst.h>
 #include <net/flow.h>
 #include <net/fib_rules.h>
@@ -273,7 +274,8 @@ static inline size_t dn_fib_nlmsg_size(struct dn_fib_info *fi)
 	size_t payload = NLMSG_ALIGN(sizeof(struct rtmsg))
 			 + nla_total_size(4) /* RTA_TABLE */
 			 + nla_total_size(2) /* RTA_DST */
-			 + nla_total_size(4); /* RTA_PRIORITY */
+			 + nla_total_size(4) /* RTA_PRIORITY */
+			 + nla_total_size(TCP_CA_NAME_MAX); /* RTAX_CC_ALGO */
 
 	/* space for nested metrics */
 	payload += nla_total_size((RTAX_MAX * nla_total_size(4)));
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index f99f41b..d2b7b55 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -360,7 +360,8 @@ static inline size_t fib_nlmsg_size(struct fib_info *fi)
 			 + nla_total_size(4) /* RTA_TABLE */
 			 + nla_total_size(4) /* RTA_DST */
 			 + nla_total_size(4) /* RTA_PRIORITY */
-			 + nla_total_size(4); /* RTA_PREFSRC */
+			 + nla_total_size(4) /* RTA_PREFSRC */
+			 + nla_total_size(TCP_CA_NAME_MAX); /* RTAX_CC_ALGO */
 
 	/* space for nested metrics */
 	payload += nla_total_size((RTAX_MAX * nla_total_size(4)));
@@ -859,7 +860,16 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
 
 				if (type > RTAX_MAX)
 					goto err_inval;
-				val = nla_get_u32(nla);
+				if (type == RTAX_CC_ALGO) {
+					char tmp[TCP_CA_NAME_MAX];
+
+					nla_strlcpy(tmp, nla, sizeof(tmp));
+					val = tcp_ca_get_key_by_name(tmp);
+					if (val == TCP_CA_UNSPEC)
+						goto err_inval;
+				} else {
+					val = nla_get_u32(nla);
+				}
 				if (type == RTAX_ADVMSS && val > 65535 - 40)
 					val = 65535 - 40;
 				if (type == RTAX_MTU && val > 65535 - 15)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index b2d1838..0998ac6 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -30,6 +30,7 @@
 #include <linux/slab.h>
 
 #include <net/ipv6.h>
+#include <net/tcp.h>
 #include <net/ndisc.h>
 #include <net/addrconf.h>
 
@@ -650,10 +651,22 @@ static int fib6_commit_metrics(struct dst_entry *dst,
 		int type = nla_type(nla);
 
 		if (type) {
+			u32 val;
+
 			if (type > RTAX_MAX)
 				return -EINVAL;
+			if (type == RTAX_CC_ALGO) {
+				char tmp[TCP_CA_NAME_MAX];
+
+				nla_strlcpy(tmp, nla, sizeof(tmp));
+				val = tcp_ca_get_key_by_name(tmp);
+				if (val == TCP_CA_UNSPEC)
+					return -EINVAL;
+			} else {
+				val = nla_get_u32(nla);
+			}
 
-			mp[type - 1] = nla_get_u32(nla);
+			mp[type - 1] = val;
 		}
 	}
 	return 0;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c910831..818c99a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2534,7 +2534,8 @@ static inline size_t rt6_nlmsg_size(void)
 	       + nla_total_size(4) /* RTA_OIF */
 	       + nla_total_size(4) /* RTA_PRIORITY */
 	       + RTAX_MAX * nla_total_size(4) /* RTA_METRICS */
-	       + nla_total_size(sizeof(struct rta_cacheinfo));
+	       + nla_total_size(sizeof(struct rta_cacheinfo))
+	       + nla_total_size(TCP_CA_NAME_MAX); /* RTAX_CC_ALGO */
 }
 
 static int rt6_fill_node(struct net *net,
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next v2 4/4] net: tcp: add per route congestion control
From: Daniel Borkmann @ 2014-12-06 23:39 UTC (permalink / raw)
  To: davem; +Cc: hannes, fw, netdev
In-Reply-To: <1417909163-19135-1-git-send-email-dborkman@redhat.com>

This work adds the possibility to define a per route/destination congestion
control algorithm. Generally, this opens up the possibility for a machine
with different links to enforce specific congestion control algorithms with
optimal strategies for each of them based on their network characteristics,
even transparently for a single application listening on all links.

For our specific use case, this additionally facilitates deployment of DCTCP,
for example, applications can easily serve internal traffic/dsts in DCTCP and
external one with CUBIC. Other scenarios would also allow for utilizing e.g.
long living, low priority background flows for certain destinations/routes
while still being able for normal traffic to utilize the default congestion
control algorithm. We also thought about a per netns setting (where different
defaults are possible), but given its actually a link specific property, we
argue that a per route/destination setting is the most natural and flexible.

The administrator can utilize this through ip-route(8) by appending "congctl
[lock] <name>", where <name> denotes the name of a loaded congestion control
algorithm and the optional lock parameter allows to enforce the given algorithm
so that applications in user space would not be allowed to overwrite that
algorithm for that destination.

The dst metric lookups are being done when a dst entry is already available
in order to avoid a costly lookup and still before the algorithms are being
initialized, thus overhead is very low when the feature is not being used.
While the client side would need to drop the current reference on the module,
on server side this can actually even be avoided as we just got a flat-copied
socket clone.

In case a dst metric contains a key to a congestion control module that has
just been removed from the kernel, it will fallback to the default congestion
control discipline for those and not reassign anything. Should at a later
point in time that module be reinserted into the kernel, then this algorithm
will be used again as keys are deterministic/static.

Joint work with Florian Westphal.

Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 include/net/tcp.h        |  6 ++++++
 net/ipv4/tcp_ipv4.c      |  2 ++
 net/ipv4/tcp_minisocks.c | 30 ++++++++++++++++++++++++++----
 net/ipv4/tcp_output.c    | 21 +++++++++++++++++++++
 net/ipv6/tcp_ipv6.c      |  2 ++
 5 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 95bb237..b8fdc6b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -448,6 +448,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb);
 struct sock *tcp_create_openreq_child(struct sock *sk,
 				      struct request_sock *req,
 				      struct sk_buff *skb);
+void tcp_ca_openreq_child(struct sock *sk, const struct dst_entry *dst);
 struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 				  struct request_sock *req,
 				  struct dst_entry *dst);
@@ -636,6 +637,11 @@ static inline u32 tcp_rto_min_us(struct sock *sk)
 	return jiffies_to_usecs(tcp_rto_min(sk));
 }
 
+static inline bool tcp_ca_dst_locked(const struct dst_entry *dst)
+{
+	return dst_metric_locked(dst, RTAX_CC_ALGO);
+}
+
 /* Compute the actual receive window we are currently advertising.
  * Rcv_nxt can be after the window if our peer push more data
  * than the offered window.
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 33f5ff0..ed94fdc 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1340,6 +1340,8 @@ struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 	}
 	sk_setup_caps(newsk, dst);
 
+	tcp_ca_openreq_child(newsk, dst);
+
 	tcp_sync_mss(newsk, dst_mtu(dst));
 	newtp->advmss = dst_metric_advmss(dst);
 	if (tcp_sk(sk)->rx_opt.user_mss &&
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 63d2680..bc9216d 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -399,6 +399,32 @@ static void tcp_ecn_openreq_child(struct tcp_sock *tp,
 	tp->ecn_flags = inet_rsk(req)->ecn_ok ? TCP_ECN_OK : 0;
 }
 
+void tcp_ca_openreq_child(struct sock *sk, const struct dst_entry *dst)
+{
+	struct inet_connection_sock *icsk = inet_csk(sk);
+	u32 ca_key = dst_metric(dst, RTAX_CC_ALGO);
+	bool ca_got_dst = false;
+
+	if (ca_key != TCP_CA_UNSPEC) {
+		const struct tcp_congestion_ops *ca;
+
+		rcu_read_lock();
+		ca = tcp_ca_find_key(ca_key);
+		if (likely(ca && try_module_get(ca->owner))) {
+			icsk->icsk_ca_dst_locked = tcp_ca_dst_locked(dst);
+			icsk->icsk_ca_ops = ca;
+			ca_got_dst = true;
+		}
+		rcu_read_unlock();
+	}
+
+	if (!ca_got_dst && !try_module_get(icsk->icsk_ca_ops->owner))
+		tcp_assign_congestion_control(sk);
+
+	tcp_set_ca_state(sk, TCP_CA_Open);
+}
+EXPORT_SYMBOL_GPL(tcp_ca_openreq_child);
+
 /* This is not only more efficient than what we used to do, it eliminates
  * a lot of code duplication between IPv4/IPv6 SYN recv processing. -DaveM
  *
@@ -451,10 +477,6 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
 		newtp->snd_cwnd = TCP_INIT_CWND;
 		newtp->snd_cwnd_cnt = 0;
 
-		if (!try_module_get(newicsk->icsk_ca_ops->owner))
-			tcp_assign_congestion_control(newsk);
-
-		tcp_set_ca_state(newsk, TCP_CA_Open);
 		tcp_init_xmit_timers(newsk);
 		__skb_queue_head_init(&newtp->out_of_order_queue);
 		newtp->write_seq = newtp->pushed_seq = treq->snt_isn + 1;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f5bd4bd..5eb5676 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2916,6 +2916,25 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
 }
 EXPORT_SYMBOL(tcp_make_synack);
 
+static void tcp_ca_dst_init(struct sock *sk, const struct dst_entry *dst)
+{
+	struct inet_connection_sock *icsk = inet_csk(sk);
+	const struct tcp_congestion_ops *ca;
+	u32 ca_key = dst_metric(dst, RTAX_CC_ALGO);
+
+	if (ca_key == TCP_CA_UNSPEC)
+		return;
+
+	rcu_read_lock();
+	ca = tcp_ca_find_key(ca_key);
+	if (likely(ca && try_module_get(ca->owner))) {
+		module_put(icsk->icsk_ca_ops->owner);
+		icsk->icsk_ca_dst_locked = tcp_ca_dst_locked(dst);
+		icsk->icsk_ca_ops = ca;
+	}
+	rcu_read_unlock();
+}
+
 /* Do all connect socket setups that can be done AF independent. */
 static void tcp_connect_init(struct sock *sk)
 {
@@ -2941,6 +2960,8 @@ static void tcp_connect_init(struct sock *sk)
 	tcp_mtup_init(sk);
 	tcp_sync_mss(sk, dst_mtu(dst));
 
+	tcp_ca_dst_init(sk, dst);
+
 	if (!tp->window_clamp)
 		tp->window_clamp = dst_metric(dst, RTAX_WINDOW);
 	tp->advmss = dst_metric_advmss(dst);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index d06af89..20f44fe 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1199,6 +1199,8 @@ static struct sock *tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 		inet_csk(newsk)->icsk_ext_hdr_len = (newnp->opt->opt_nflen +
 						     newnp->opt->opt_flen);
 
+	tcp_ca_openreq_child(newsk, dst);
+
 	tcp_sync_mss(newsk, dst_mtu(dst));
 	newtp->advmss = dst_metric_advmss(dst);
 	if (tcp_sk(sk)->rx_opt.user_mss &&
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next v2 2/4] net: tcp: add key management to congestion control
From: Daniel Borkmann @ 2014-12-06 23:39 UTC (permalink / raw)
  To: davem; +Cc: hannes, fw, netdev
In-Reply-To: <1417909163-19135-1-git-send-email-dborkman@redhat.com>

For a per-route congestion control possibility, our aim is to store
a unique u32 key identifier into dst metrics, which can then be
mapped into a tcp_congestion_ops struct. We argue that having a
RTAX key entry is the most simple, generic and easy way to manage,
and also keeps the memory footprint of dst entries lower on 64 bit
than with storing a pointer directly, for example. Having a unique
key id also allows for decoupling actual TCP congestion control
module management from the FIB layer, i.e. we don't have to care
about expensive module refcounting inside the FIB at this point.

We first thought of using an IDR store for the realization, which
takes over dynamic assignment of unused key space and also performs
the key to pointer mapping in RCU. While doing so, we stumbled upon
the issue that due to the nature of dynamic key distribution, it
just so happens, arguably in very rare occasions, that excessive
module loads and unloads can lead to a possible reuse of previously
used key space. Thus, previously stale keys in the dst metric are
now being reassigned to a different congestion control algorithm,
which might lead to unexpected behaviour. One way to resolve this
would have been to walk FIBs on the actually rare occasion of a
module unload and reset the metric keys for each FIB in each netns,
but that's just very costly.

Therefore, we argue a better solution is to reuse the unique
congestion control algorithm name member and map that into u32 key
space through jhash. For that, we split the flags attribute (as it
currently uses 2 bits only anyway) into two u32 attributes, flags
and key, so that we can keep the cacheline boundary of 2 cachelines
on x86_64 and cache the precalculated key at registration time for
the fast path. On average we might expect 2 - 4 modules being loaded
worst case perhaps 15, so a key collision possibility is extremely
low, and guaranteed collision-free on LE/BE for all in-tree modules.
Overall this results in much simpler code, and all without the
overhead of an IDR. Due to the deterministic nature, modules can
now be unloaded, the congestion control algorithm for a specific
but unloaded key will fall back to the default one, and on module
reload time it will switch back to the expected algorithm
transparently.

Joint work with Florian Westphal.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 include/net/inet_connection_sock.h |  3 +-
 include/net/tcp.h                  |  9 +++++-
 net/ipv4/tcp_cong.c                | 63 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 848e85c..5976bde 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -98,7 +98,8 @@ struct inet_connection_sock {
 	const struct tcp_congestion_ops *icsk_ca_ops;
 	const struct inet_connection_sock_af_ops *icsk_af_ops;
 	unsigned int		  (*icsk_sync_mss)(struct sock *sk, u32 pmtu);
-	__u8			  icsk_ca_state;
+	__u8			  icsk_ca_state:7,
+				  icsk_ca_dst_locked:1;
 	__u8			  icsk_retransmits;
 	__u8			  icsk_pending;
 	__u8			  icsk_backoff;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index f50f29faf..135b70c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -787,6 +787,8 @@ enum tcp_ca_ack_event_flags {
 #define TCP_CA_MAX	128
 #define TCP_CA_BUF_MAX	(TCP_CA_NAME_MAX*TCP_CA_MAX)
 
+#define TCP_CA_UNSPEC	0
+
 /* Algorithm can be set on socket without CAP_NET_ADMIN privileges */
 #define TCP_CONG_NON_RESTRICTED 0x1
 /* Requires ECN/ECT set on all packets */
@@ -794,7 +796,8 @@ enum tcp_ca_ack_event_flags {
 
 struct tcp_congestion_ops {
 	struct list_head	list;
-	unsigned long flags;
+	u32 key;
+	u32 flags;
 
 	/* initialize private data (optional) */
 	void (*init)(struct sock *sk);
@@ -841,6 +844,10 @@ u32 tcp_reno_ssthresh(struct sock *sk);
 void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 acked);
 extern struct tcp_congestion_ops tcp_reno;
 
+struct tcp_congestion_ops *tcp_ca_find_key(u32 key);
+u32 tcp_ca_get_key_by_name(const char *name);
+char *tcp_ca_get_name_by_key(u32 key, char *buffer);
+
 static inline bool tcp_ca_needs_ecn(const struct sock *sk)
 {
 	const struct inet_connection_sock *icsk = inet_csk(sk);
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 38f2f8a..8fd348c 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -13,6 +13,7 @@
 #include <linux/types.h>
 #include <linux/list.h>
 #include <linux/gfp.h>
+#include <linux/jhash.h>
 #include <net/tcp.h>
 
 static DEFINE_SPINLOCK(tcp_cong_list_lock);
@@ -31,6 +32,19 @@ static struct tcp_congestion_ops *tcp_ca_find(const char *name)
 	return NULL;
 }
 
+/* Simple linear search, not much in here. */
+struct tcp_congestion_ops *tcp_ca_find_key(u32 key)
+{
+	struct tcp_congestion_ops *e;
+
+	list_for_each_entry_rcu(e, &tcp_cong_list, list) {
+		if (e->key == key)
+			return e;
+	}
+
+	return NULL;
+}
+
 /*
  * Attach new congestion control algorithm to the list
  * of available options.
@@ -45,9 +59,12 @@ int tcp_register_congestion_control(struct tcp_congestion_ops *ca)
 		return -EINVAL;
 	}
 
+	ca->key = jhash(ca->name, sizeof(ca->name), strlen(ca->name));
+
 	spin_lock(&tcp_cong_list_lock);
-	if (tcp_ca_find(ca->name)) {
-		pr_notice("%s already registered\n", ca->name);
+	if (ca->key == TCP_CA_UNSPEC || tcp_ca_find_key(ca->key)) {
+		pr_notice("%s already registered or non-unique key\n",
+			  ca->name);
 		ret = -EEXIST;
 	} else {
 		list_add_tail_rcu(&ca->list, &tcp_cong_list);
@@ -70,9 +87,48 @@ void tcp_unregister_congestion_control(struct tcp_congestion_ops *ca)
 	spin_lock(&tcp_cong_list_lock);
 	list_del_rcu(&ca->list);
 	spin_unlock(&tcp_cong_list_lock);
+
+	/* Wait for outstanding readers to complete before the
+	 * module gets removed entirely.
+	 *
+	 * A try_module_get() should fail by now as our module is
+	 * in "going" state since no refs are held anymore and
+	 * module_exit() handler being called.
+	 */
+	synchronize_rcu();
 }
 EXPORT_SYMBOL_GPL(tcp_unregister_congestion_control);
 
+u32 tcp_ca_get_key_by_name(const char *name)
+{
+	const struct tcp_congestion_ops *ca;
+	u32 key;
+
+	rcu_read_lock();
+	ca = tcp_ca_find(name);
+	key = ca ? ca->key : TCP_CA_UNSPEC;
+	rcu_read_unlock();
+
+	return key;
+}
+EXPORT_SYMBOL_GPL(tcp_ca_get_key_by_name);
+
+char *tcp_ca_get_name_by_key(u32 key, char *buffer)
+{
+	const struct tcp_congestion_ops *ca;
+	char *ret = NULL;
+
+	rcu_read_lock();
+	ca = tcp_ca_find_key(key);
+	if (ca)
+		ret = strncpy(buffer, ca->name,
+			      TCP_CA_NAME_MAX);
+	rcu_read_unlock();
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tcp_ca_get_name_by_key);
+
 /* Assign choice of congestion control. */
 void tcp_assign_congestion_control(struct sock *sk)
 {
@@ -256,6 +312,9 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
 	struct tcp_congestion_ops *ca;
 	int err = 0;
 
+	if (icsk->icsk_ca_dst_locked)
+		return -EPERM;
+
 	rcu_read_lock();
 	ca = tcp_ca_find(name);
 
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH net-next 1/4] netdevice: Add a function to check macvlan port
From: Mahesh Bandewar @ 2014-12-06 23:53 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Mahesh Bandewar

Similar to a check for macvlan device, netif_is_macvlan(), add
another function to check if a device is used as macvlan port.

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 include/linux/netdevice.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 29c92ee9ed56..1f49aac258f9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3641,6 +3641,11 @@ static inline bool netif_is_macvlan(struct net_device *dev)
 	return dev->priv_flags & IFF_MACVLAN;
 }
 
+static inline bool netif_is_macvlan_port(struct net_device *dev)
+{
+	return dev->priv_flags & IFF_MACVLAN_PORT;
+}
+
 static inline bool netif_is_bond_master(struct net_device *dev)
 {
 	return dev->flags & IFF_MASTER && dev->priv_flags & IFF_BONDING;
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next 2/4] ipvlan: play well with macvlan device
From: Mahesh Bandewar @ 2014-12-06 23:53 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Mahesh Bandewar

If a device is already a macvlan port then refuse to use it as
an ipvlan port in the early stage of port creation.

	thost1:~# ip link add link eth0 mvl0 type macvlan
	thost1:~# echo $?
	0
	thost1:~# ip link add link eth0 ipvl0 type ipvlan
	RTNETLINK answers: Device or resource busy
	thost1:~# echo $?
	2
	thost1:~#

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/ipvlan/ipvlan_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 96b71b0d78f6..a706a547d811 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -38,6 +38,12 @@ static int ipvlan_port_create(struct net_device *dev)
 		netdev_err(dev, "Master is either lo or non-ether device\n");
 		return -EINVAL;
 	}
+
+	if (netif_is_macvlan_port(dev)) {
+		netdev_err(dev, "Master is a macvlan port.\n");
+		return -EBUSY;
+	}
+
 	port = kzalloc(sizeof(struct ipvl_port), GFP_KERNEL);
 	if (!port)
 		return -ENOMEM;
-- 
2.2.0.rc0.207.ga3a616c

^ 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