Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 3/9] nfp: support LSO2 capability
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Edwin Peer, Jakub Kicinski
In-Reply-To: <20170516005523.26124-1-jakub.kicinski@netronome.com>

From: Edwin Peer <edwin.peer@netronome.com>

Firmware advertising the LSO2 capability exploits driver provided L3 and L4
offsets in order to avoid parsing packet headers in the TX path. The vlan
field in struct nfp_net_tx_desc is repurposed, making TXVLAN a mutually
exclusive configuration to LSO2.

Signed-off-by: Edwin Peer <edwin.peer@netronome.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net.h       |  9 +++--
 .../net/ethernet/netronome/nfp/nfp_net_common.c    | 38 ++++++++++++++--------
 drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h  |  7 +++-
 3 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index 6bad11e5b845..c6b7141dc50d 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -155,8 +155,13 @@ struct nfp_net_tx_desc {
 			__le16 mss;	/* MSS to be used for LSO */
 			u8 lso_hdrlen;	/* LSO, TCP payload offset */
 			u8 flags;	/* TX Flags, see @PCIE_DESC_TX_* */
-
-			__le16 vlan;	/* VLAN tag to add if indicated */
+			union {
+				struct {
+					u8 l3_offset; /* L3 header offset */
+					u8 l4_offset; /* L4 header offset */
+				};
+				__le16 vlan; /* VLAN tag to add if indicated */
+			};
 			__le16 data_len; /* Length of frame + meta data */
 		} __packed;
 		__le32 vals[4];
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 0cebe9098451..5e8049a84d16 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -667,11 +667,16 @@ static void nfp_net_tx_tso(struct nfp_net_r_vector *r_vec,
 	if (!skb_is_gso(skb))
 		return;
 
-	if (!skb->encapsulation)
+	if (!skb->encapsulation) {
+		txd->l3_offset = skb_network_offset(skb);
+		txd->l4_offset = skb_transport_offset(skb);
 		hdrlen = skb_transport_offset(skb) + tcp_hdrlen(skb);
-	else
+	} else {
+		txd->l3_offset = skb_inner_network_offset(skb);
+		txd->l4_offset = skb_inner_transport_offset(skb);
 		hdrlen = skb_inner_transport_header(skb) - skb->data +
 			inner_tcp_hdrlen(skb);
+	}
 
 	txbuf->pkt_cnt = skb_shinfo(skb)->gso_segs;
 	txbuf->real_len += hdrlen * (txbuf->pkt_cnt - 1);
@@ -825,10 +830,9 @@ static int nfp_net_tx(struct sk_buff *skb, struct net_device *netdev)
 	txd->mss = 0;
 	txd->lso_hdrlen = 0;
 
+	/* Do not reorder - tso may adjust pkt cnt, vlan may override fields */
 	nfp_net_tx_tso(r_vec, txbuf, txd, skb);
-
 	nfp_net_tx_csum(dp, r_vec, txbuf, txd, skb);
-
 	if (skb_vlan_tag_present(skb) && dp->ctrl & NFP_NET_CFG_CTRL_TXVLAN) {
 		txd->flags |= PCIE_DESC_TX_VLAN;
 		txd->vlan = cpu_to_le16(skb_vlan_tag_get(skb));
@@ -2724,9 +2728,10 @@ static int nfp_net_set_features(struct net_device *netdev,
 
 	if (changed & (NETIF_F_TSO | NETIF_F_TSO6)) {
 		if (features & (NETIF_F_TSO | NETIF_F_TSO6))
-			new_ctrl |= NFP_NET_CFG_CTRL_LSO;
+			new_ctrl |= nn->cap & NFP_NET_CFG_CTRL_LSO2 ?:
+					      NFP_NET_CFG_CTRL_LSO;
 		else
-			new_ctrl &= ~NFP_NET_CFG_CTRL_LSO;
+			new_ctrl &= ~NFP_NET_CFG_CTRL_LSO_ANY;
 	}
 
 	if (changed & NETIF_F_HW_VLAN_CTAG_RX) {
@@ -3032,7 +3037,7 @@ void nfp_net_info(struct nfp_net *nn)
 		nn->fw_ver.resv, nn->fw_ver.class,
 		nn->fw_ver.major, nn->fw_ver.minor,
 		nn->max_mtu);
-	nn_info(nn, "CAP: %#x %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
+	nn_info(nn, "CAP: %#x %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
 		nn->cap,
 		nn->cap & NFP_NET_CFG_CTRL_PROMISC  ? "PROMISC "  : "",
 		nn->cap & NFP_NET_CFG_CTRL_L2BC     ? "L2BCFILT " : "",
@@ -3043,7 +3048,8 @@ void nfp_net_info(struct nfp_net *nn)
 		nn->cap & NFP_NET_CFG_CTRL_TXVLAN   ? "TXVLAN "   : "",
 		nn->cap & NFP_NET_CFG_CTRL_SCATTER  ? "SCATTER "  : "",
 		nn->cap & NFP_NET_CFG_CTRL_GATHER   ? "GATHER "   : "",
-		nn->cap & NFP_NET_CFG_CTRL_LSO      ? "TSO "      : "",
+		nn->cap & NFP_NET_CFG_CTRL_LSO      ? "TSO1 "     : "",
+		nn->cap & NFP_NET_CFG_CTRL_LSO2     ? "TSO2 "     : "",
 		nn->cap & NFP_NET_CFG_CTRL_RSS      ? "RSS "      : "",
 		nn->cap & NFP_NET_CFG_CTRL_L2SWITCH ? "L2SWITCH " : "",
 		nn->cap & NFP_NET_CFG_CTRL_MSIXAUTO ? "AUTOMASK " : "",
@@ -3249,9 +3255,11 @@ int nfp_net_netdev_init(struct net_device *netdev)
 		netdev->hw_features |= NETIF_F_SG;
 		nn->dp.ctrl |= NFP_NET_CFG_CTRL_GATHER;
 	}
-	if ((nn->cap & NFP_NET_CFG_CTRL_LSO) && nn->fw_ver.major > 2) {
+	if ((nn->cap & NFP_NET_CFG_CTRL_LSO && nn->fw_ver.major > 2) ||
+	    nn->cap & NFP_NET_CFG_CTRL_LSO2) {
 		netdev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
-		nn->dp.ctrl |= NFP_NET_CFG_CTRL_LSO;
+		nn->dp.ctrl |= nn->cap & NFP_NET_CFG_CTRL_LSO2 ?:
+					 NFP_NET_CFG_CTRL_LSO;
 	}
 	if (nn->cap & NFP_NET_CFG_CTRL_RSS) {
 		netdev->hw_features |= NETIF_F_RXHASH;
@@ -3275,8 +3283,12 @@ int nfp_net_netdev_init(struct net_device *netdev)
 		nn->dp.ctrl |= NFP_NET_CFG_CTRL_RXVLAN;
 	}
 	if (nn->cap & NFP_NET_CFG_CTRL_TXVLAN) {
-		netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX;
-		nn->dp.ctrl |= NFP_NET_CFG_CTRL_TXVLAN;
+		if (nn->cap & NFP_NET_CFG_CTRL_LSO2) {
+			nn_warn(nn, "Device advertises both TSO2 and TXVLAN. Refusing to enable TXVLAN.\n");
+		} else {
+			netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX;
+			nn->dp.ctrl |= NFP_NET_CFG_CTRL_TXVLAN;
+		}
 	}
 
 	netdev->features = netdev->hw_features;
@@ -3286,7 +3298,7 @@ int nfp_net_netdev_init(struct net_device *netdev)
 
 	/* Advertise but disable TSO by default. */
 	netdev->features &= ~(NETIF_F_TSO | NETIF_F_TSO6);
-	nn->dp.ctrl &= ~NFP_NET_CFG_CTRL_LSO;
+	nn->dp.ctrl &= ~NFP_NET_CFG_CTRL_LSO_ANY;
 
 	/* Allow L2 Broadcast and Multicast through by default, if supported */
 	if (nn->cap & NFP_NET_CFG_CTRL_L2BC)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h b/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
index d04ccc9f6116..1575e8fdb541 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
@@ -119,7 +119,7 @@
 #define   NFP_NET_CFG_CTRL_TXVLAN         (0x1 <<  7) /* Enable VLAN insert */
 #define   NFP_NET_CFG_CTRL_SCATTER        (0x1 <<  8) /* Scatter DMA */
 #define   NFP_NET_CFG_CTRL_GATHER         (0x1 <<  9) /* Gather DMA */
-#define   NFP_NET_CFG_CTRL_LSO            (0x1 << 10) /* LSO/TSO */
+#define   NFP_NET_CFG_CTRL_LSO            (0x1 << 10) /* LSO/TSO (version 1) */
 #define   NFP_NET_CFG_CTRL_RINGCFG        (0x1 << 16) /* Ring runtime changes */
 #define   NFP_NET_CFG_CTRL_RSS            (0x1 << 17) /* RSS */
 #define   NFP_NET_CFG_CTRL_IRQMOD         (0x1 << 18) /* Interrupt moderation */
@@ -131,6 +131,11 @@
 #define   NFP_NET_CFG_CTRL_VXLAN	  (0x1 << 24) /* VXLAN tunnel support */
 #define   NFP_NET_CFG_CTRL_NVGRE	  (0x1 << 25) /* NVGRE tunnel support */
 #define   NFP_NET_CFG_CTRL_BPF		  (0x1 << 27) /* BPF offload capable */
+#define   NFP_NET_CFG_CTRL_LSO2		  (0x1 << 28) /* LSO/TSO (version 2) */
+
+#define NFP_NET_CFG_CTRL_LSO_ANY	(NFP_NET_CFG_CTRL_LSO | \
+					 NFP_NET_CFG_CTRL_LSO2)
+
 #define NFP_NET_CFG_UPDATE              0x0004
 #define   NFP_NET_CFG_UPDATE_GEN          (0x1 <<  0) /* General update */
 #define   NFP_NET_CFG_UPDATE_RING         (0x1 <<  1) /* Ring config change */
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 9/9] nfp: eliminate an if statement in calculation of completed frames
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski
In-Reply-To: <20170516005523.26124-1-jakub.kicinski@netronome.com>

Given that our rings are always a power of 2, we can simplify the
calculation of number of completed TX descriptors by using masking
instead of if statement based on whether the index have wrapped
or not.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index c64514f8ee65..da83e17b8b20 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -940,10 +940,7 @@ static void nfp_net_tx_complete(struct nfp_net_tx_ring *tx_ring)
 	if (qcp_rd_p == tx_ring->qcp_rd_p)
 		return;
 
-	if (qcp_rd_p > tx_ring->qcp_rd_p)
-		todo = qcp_rd_p - tx_ring->qcp_rd_p;
-	else
-		todo = qcp_rd_p + tx_ring->cnt - tx_ring->qcp_rd_p;
+	todo = D_IDX(tx_ring, qcp_rd_p + tx_ring->cnt - tx_ring->qcp_rd_p);
 
 	while (todo--) {
 		idx = D_IDX(tx_ring, tx_ring->rd_p++);
@@ -1014,10 +1011,7 @@ static bool nfp_net_xdp_complete(struct nfp_net_tx_ring *tx_ring)
 	if (qcp_rd_p == tx_ring->qcp_rd_p)
 		return true;
 
-	if (qcp_rd_p > tx_ring->qcp_rd_p)
-		todo = qcp_rd_p - tx_ring->qcp_rd_p;
-	else
-		todo = qcp_rd_p + tx_ring->cnt - tx_ring->qcp_rd_p;
+	todo = D_IDX(tx_ring, qcp_rd_p + tx_ring->cnt - tx_ring->qcp_rd_p);
 
 	done_all = todo <= NFP_NET_XDP_MAX_COMPLETE;
 	todo = min(todo, NFP_NET_XDP_MAX_COMPLETE);
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 4/9] nfp: don't assume RSS and IRQ moderation are always enabled
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski
In-Reply-To: <20170516005523.26124-1-jakub.kicinski@netronome.com>

Even if capability for RSS and IRQ moderation are present we may
have not initialized them for control vNIC.  Depend on selected
features mask (ctrl) rather than capabilities (cap) to determine
which features should be enabled.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 5e8049a84d16..ae32c0e8d6e6 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -2201,17 +2201,15 @@ static int nfp_net_set_config_and_enable(struct nfp_net *nn)
 
 	new_ctrl = nn->dp.ctrl;
 
-	if (nn->cap & NFP_NET_CFG_CTRL_RSS) {
+	if (nn->dp.ctrl & NFP_NET_CFG_CTRL_RSS) {
 		nfp_net_rss_write_key(nn);
 		nfp_net_rss_write_itbl(nn);
 		nn_writel(nn, NFP_NET_CFG_RSS_CTRL, nn->rss_cfg);
 		update |= NFP_NET_CFG_UPDATE_RSS;
 	}
 
-	if (nn->cap & NFP_NET_CFG_CTRL_IRQMOD) {
+	if (nn->dp.ctrl & NFP_NET_CFG_CTRL_IRQMOD) {
 		nfp_net_coalesce_write_cfg(nn);
-
-		new_ctrl |= NFP_NET_CFG_CTRL_IRQMOD;
 		update |= NFP_NET_CFG_UPDATE_IRQMOD;
 	}
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 5/9] nfp: version independent support for chained RSS metadata
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Edwin Peer
In-Reply-To: <20170516005523.26124-1-jakub.kicinski@netronome.com>

From: Edwin Peer <edwin.peer@netronome.com>

ABI version 4 introduced metadata chaining. Using the ABI version to signal
metadata chaining precludes firmware that advertises new capabilities which
rely on prepended metadata from working on older kernels.

Capability bits are thus better suited to signalling the chained metadata
format. A new version of the RSS capability is introduced to distinguish
between the differing metadata formats for ABI versions other than 4.

Signed-off-by: Edwin Peer <edwin.peer@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c  | 20 +++++++++++++-------
 drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h    |  6 +++++-
 drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c | 12 ++++++------
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index ae32c0e8d6e6..cc5a2eaef156 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -2201,7 +2201,7 @@ static int nfp_net_set_config_and_enable(struct nfp_net *nn)
 
 	new_ctrl = nn->dp.ctrl;
 
-	if (nn->dp.ctrl & NFP_NET_CFG_CTRL_RSS) {
+	if (nn->dp.ctrl & NFP_NET_CFG_CTRL_RSS_ANY) {
 		nfp_net_rss_write_key(nn);
 		nfp_net_rss_write_itbl(nn);
 		nn_writel(nn, NFP_NET_CFG_RSS_CTRL, nn->rss_cfg);
@@ -3035,7 +3035,7 @@ void nfp_net_info(struct nfp_net *nn)
 		nn->fw_ver.resv, nn->fw_ver.class,
 		nn->fw_ver.major, nn->fw_ver.minor,
 		nn->max_mtu);
-	nn_info(nn, "CAP: %#x %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
+	nn_info(nn, "CAP: %#x %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
 		nn->cap,
 		nn->cap & NFP_NET_CFG_CTRL_PROMISC  ? "PROMISC "  : "",
 		nn->cap & NFP_NET_CFG_CTRL_L2BC     ? "L2BCFILT " : "",
@@ -3048,7 +3048,8 @@ void nfp_net_info(struct nfp_net *nn)
 		nn->cap & NFP_NET_CFG_CTRL_GATHER   ? "GATHER "   : "",
 		nn->cap & NFP_NET_CFG_CTRL_LSO      ? "TSO1 "     : "",
 		nn->cap & NFP_NET_CFG_CTRL_LSO2     ? "TSO2 "     : "",
-		nn->cap & NFP_NET_CFG_CTRL_RSS      ? "RSS "      : "",
+		nn->cap & NFP_NET_CFG_CTRL_RSS      ? "RSS1 "     : "",
+		nn->cap & NFP_NET_CFG_CTRL_RSS2     ? "RSS2 "     : "",
 		nn->cap & NFP_NET_CFG_CTRL_L2SWITCH ? "L2SWITCH " : "",
 		nn->cap & NFP_NET_CFG_CTRL_MSIXAUTO ? "AUTOMASK " : "",
 		nn->cap & NFP_NET_CFG_CTRL_IRQMOD   ? "IRQMOD "   : "",
@@ -3202,14 +3203,18 @@ int nfp_net_netdev_init(struct net_device *netdev)
 	struct nfp_net *nn = netdev_priv(netdev);
 	int err;
 
-	nn->dp.chained_metadata_format = nn->fw_ver.major > 3;
-
 	nn->dp.rx_dma_dir = DMA_FROM_DEVICE;
 
 	/* Get some of the read-only fields from the BAR */
 	nn->cap = nn_readl(nn, NFP_NET_CFG_CAP);
 	nn->max_mtu = nn_readl(nn, NFP_NET_CFG_MAX_MTU);
 
+	/* Chained metadata is signalled by capabilities except in version 4 */
+	nn->dp.chained_metadata_format = nn->fw_ver.major == 4 ||
+					 nn->cap & NFP_NET_CFG_CTRL_CHAIN_META;
+	if (nn->dp.chained_metadata_format && nn->fw_ver.major != 4)
+		nn->cap &= ~NFP_NET_CFG_CTRL_RSS;
+
 	nfp_net_write_mac_addr(nn);
 
 	/* Determine RX packet/metadata boundary offset */
@@ -3259,10 +3264,11 @@ int nfp_net_netdev_init(struct net_device *netdev)
 		nn->dp.ctrl |= nn->cap & NFP_NET_CFG_CTRL_LSO2 ?:
 					 NFP_NET_CFG_CTRL_LSO;
 	}
-	if (nn->cap & NFP_NET_CFG_CTRL_RSS) {
+	if (nn->cap & NFP_NET_CFG_CTRL_RSS_ANY) {
 		netdev->hw_features |= NETIF_F_RXHASH;
 		nfp_net_rss_init(nn);
-		nn->dp.ctrl |= NFP_NET_CFG_CTRL_RSS;
+		nn->dp.ctrl |= nn->cap & NFP_NET_CFG_CTRL_RSS2 ?:
+					 NFP_NET_CFG_CTRL_RSS;
 	}
 	if (nn->cap & NFP_NET_CFG_CTRL_VXLAN &&
 	    nn->cap & NFP_NET_CFG_CTRL_NVGRE) {
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h b/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
index 1575e8fdb541..a049c5d6839d 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h
@@ -121,7 +121,7 @@
 #define   NFP_NET_CFG_CTRL_GATHER         (0x1 <<  9) /* Gather DMA */
 #define   NFP_NET_CFG_CTRL_LSO            (0x1 << 10) /* LSO/TSO (version 1) */
 #define   NFP_NET_CFG_CTRL_RINGCFG        (0x1 << 16) /* Ring runtime changes */
-#define   NFP_NET_CFG_CTRL_RSS            (0x1 << 17) /* RSS */
+#define   NFP_NET_CFG_CTRL_RSS		  (0x1 << 17) /* RSS (version 1) */
 #define   NFP_NET_CFG_CTRL_IRQMOD         (0x1 << 18) /* Interrupt moderation */
 #define   NFP_NET_CFG_CTRL_RINGPRIO       (0x1 << 19) /* Ring priorities */
 #define   NFP_NET_CFG_CTRL_MSIXAUTO       (0x1 << 20) /* MSI-X auto-masking */
@@ -132,9 +132,13 @@
 #define   NFP_NET_CFG_CTRL_NVGRE	  (0x1 << 25) /* NVGRE tunnel support */
 #define   NFP_NET_CFG_CTRL_BPF		  (0x1 << 27) /* BPF offload capable */
 #define   NFP_NET_CFG_CTRL_LSO2		  (0x1 << 28) /* LSO/TSO (version 2) */
+#define   NFP_NET_CFG_CTRL_RSS2		  (0x1 << 29) /* RSS (version 2) */
 
 #define NFP_NET_CFG_CTRL_LSO_ANY	(NFP_NET_CFG_CTRL_LSO | \
 					 NFP_NET_CFG_CTRL_LSO2)
+#define NFP_NET_CFG_CTRL_RSS_ANY	(NFP_NET_CFG_CTRL_RSS | \
+					 NFP_NET_CFG_CTRL_RSS2)
+#define NFP_NET_CFG_CTRL_CHAIN_META	NFP_NET_CFG_CTRL_RSS2
 
 #define NFP_NET_CFG_UPDATE              0x0004
 #define   NFP_NET_CFG_UPDATE_GEN          (0x1 <<  0) /* General update */
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index abbb47e60cc3..70bb0a0152b9 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -496,7 +496,7 @@ static int nfp_net_get_rss_hash_opts(struct nfp_net *nn,
 
 	cmd->data = 0;
 
-	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS))
+	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS_ANY))
 		return -EOPNOTSUPP;
 
 	nfp_rss_flag = ethtool_flow_to_nfp_flag(cmd->flow_type);
@@ -533,7 +533,7 @@ static int nfp_net_set_rss_hash_opt(struct nfp_net *nn,
 	u32 nfp_rss_flag;
 	int err;
 
-	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS))
+	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS_ANY))
 		return -EOPNOTSUPP;
 
 	/* RSS only supports IP SA/DA and L4 src/dst ports  */
@@ -595,7 +595,7 @@ static u32 nfp_net_get_rxfh_indir_size(struct net_device *netdev)
 {
 	struct nfp_net *nn = netdev_priv(netdev);
 
-	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS))
+	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS_ANY))
 		return 0;
 
 	return ARRAY_SIZE(nn->rss_itbl);
@@ -605,7 +605,7 @@ static u32 nfp_net_get_rxfh_key_size(struct net_device *netdev)
 {
 	struct nfp_net *nn = netdev_priv(netdev);
 
-	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS))
+	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS_ANY))
 		return -EOPNOTSUPP;
 
 	return nfp_net_rss_key_sz(nn);
@@ -617,7 +617,7 @@ static int nfp_net_get_rxfh(struct net_device *netdev, u32 *indir, u8 *key,
 	struct nfp_net *nn = netdev_priv(netdev);
 	int i;
 
-	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS))
+	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS_ANY))
 		return -EOPNOTSUPP;
 
 	if (indir)
@@ -641,7 +641,7 @@ static int nfp_net_set_rxfh(struct net_device *netdev,
 	struct nfp_net *nn = netdev_priv(netdev);
 	int i;
 
-	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS) ||
+	if (!(nn->cap & NFP_NET_CFG_CTRL_RSS_ANY) ||
 	    !(hfunc == ETH_RSS_HASH_NO_CHANGE || hfunc == nn->rss_hfunc))
 		return -EOPNOTSUPP;
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 8/9] nfp: add a helper for wrapping descriptor index
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski
In-Reply-To: <20170516005523.26124-1-jakub.kicinski@netronome.com>

We have a number of places where we calculate the descriptor
index based on a value which may have overflown.  Create a
macro for masking with the ring size.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net.h        |  3 +++
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 21 ++++++++++-----------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index 66319a1026bb..7b9518cbe965 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -117,6 +117,9 @@ struct nfp_eth_table_port;
 struct nfp_net;
 struct nfp_net_r_vector;
 
+/* Convenience macro for wrapping descriptor index on ring size */
+#define D_IDX(ring, idx)	((idx) & ((ring)->cnt - 1))
+
 /* Convenience macro for writing dma address into RX/TX descriptors */
 #define nfp_desc_set_dma_addr(desc, dma_addr)				\
 	do {								\
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index a4c6878f1df1..c64514f8ee65 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -809,7 +809,7 @@ static int nfp_net_tx(struct sk_buff *skb, struct net_device *netdev)
 	if (dma_mapping_error(dp->dev, dma_addr))
 		goto err_free;
 
-	wr_idx = tx_ring->wr_p & (tx_ring->cnt - 1);
+	wr_idx = D_IDX(tx_ring, tx_ring->wr_p);
 
 	/* Stash the soft descriptor of the head then initialize it */
 	txbuf = &tx_ring->txbufs[wr_idx];
@@ -852,7 +852,7 @@ static int nfp_net_tx(struct sk_buff *skb, struct net_device *netdev)
 			if (dma_mapping_error(dp->dev, dma_addr))
 				goto err_unmap;
 
-			wr_idx = (wr_idx + 1) & (tx_ring->cnt - 1);
+			wr_idx = D_IDX(tx_ring, wr_idx + 1);
 			tx_ring->txbufs[wr_idx].skb = skb;
 			tx_ring->txbufs[wr_idx].dma_addr = dma_addr;
 			tx_ring->txbufs[wr_idx].fidx = f;
@@ -946,8 +946,7 @@ static void nfp_net_tx_complete(struct nfp_net_tx_ring *tx_ring)
 		todo = qcp_rd_p + tx_ring->cnt - tx_ring->qcp_rd_p;
 
 	while (todo--) {
-		idx = tx_ring->rd_p & (tx_ring->cnt - 1);
-		tx_ring->rd_p++;
+		idx = D_IDX(tx_ring, tx_ring->rd_p++);
 
 		skb = tx_ring->txbufs[idx].skb;
 		if (!skb)
@@ -1023,11 +1022,11 @@ static bool nfp_net_xdp_complete(struct nfp_net_tx_ring *tx_ring)
 	done_all = todo <= NFP_NET_XDP_MAX_COMPLETE;
 	todo = min(todo, NFP_NET_XDP_MAX_COMPLETE);
 
-	tx_ring->qcp_rd_p = (tx_ring->qcp_rd_p + todo) & (tx_ring->cnt - 1);
+	tx_ring->qcp_rd_p = D_IDX(tx_ring, tx_ring->qcp_rd_p + todo);
 
 	done_pkts = todo;
 	while (todo--) {
-		idx = tx_ring->rd_p & (tx_ring->cnt - 1);
+		idx = D_IDX(tx_ring, tx_ring->rd_p);
 		tx_ring->rd_p++;
 
 		done_bytes += tx_ring->txbufs[idx].real_len;
@@ -1063,7 +1062,7 @@ nfp_net_tx_ring_reset(struct nfp_net_dp *dp, struct nfp_net_tx_ring *tx_ring)
 		struct sk_buff *skb;
 		int idx, nr_frags;
 
-		idx = tx_ring->rd_p & (tx_ring->cnt - 1);
+		idx = D_IDX(tx_ring, tx_ring->rd_p);
 		tx_buf = &tx_ring->txbufs[idx];
 
 		skb = tx_ring->txbufs[idx].skb;
@@ -1216,7 +1215,7 @@ static void nfp_net_rx_give_one(const struct nfp_net_dp *dp,
 {
 	unsigned int wr_idx;
 
-	wr_idx = rx_ring->wr_p & (rx_ring->cnt - 1);
+	wr_idx = D_IDX(rx_ring, rx_ring->wr_p);
 
 	nfp_net_dma_sync_dev_rx(dp, dma_addr);
 
@@ -1254,7 +1253,7 @@ static void nfp_net_rx_ring_reset(struct nfp_net_rx_ring *rx_ring)
 	unsigned int wr_idx, last_idx;
 
 	/* Move the empty entry to the end of the list */
-	wr_idx = rx_ring->wr_p & (rx_ring->cnt - 1);
+	wr_idx = D_IDX(rx_ring, rx_ring->wr_p);
 	last_idx = rx_ring->cnt - 1;
 	rx_ring->rxbufs[wr_idx].dma_addr = rx_ring->rxbufs[last_idx].dma_addr;
 	rx_ring->rxbufs[wr_idx].frag = rx_ring->rxbufs[last_idx].frag;
@@ -1522,7 +1521,7 @@ nfp_net_tx_xdp_buf(struct nfp_net_dp *dp, struct nfp_net_rx_ring *rx_ring,
 		}
 	}
 
-	wr_idx = tx_ring->wr_p & (tx_ring->cnt - 1);
+	wr_idx = D_IDX(tx_ring, tx_ring->wr_p);
 
 	/* Stash the soft descriptor of the head then initialize it */
 	txbuf = &tx_ring->txbufs[wr_idx];
@@ -1610,7 +1609,7 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget)
 		dma_addr_t new_dma_addr;
 		void *new_frag;
 
-		idx = rx_ring->rd_p & (rx_ring->cnt - 1);
+		idx = D_IDX(rx_ring, rx_ring->rd_p);
 
 		rxd = &rx_ring->rxds[idx];
 		if (!(rxd->rxd.meta_len_dd & PCIE_DESC_RX_DD))
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 7/9] nfp: complete the XDP TX ring only when it's full
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski
In-Reply-To: <20170516005523.26124-1-jakub.kicinski@netronome.com>

Since XDP TX ring holds "spare" RX buffers anyway, we don't have to
rush the completion.  We can wait until ring fills up completely
before trying to reclaim buffers.  If RX poll has ended an no
buffer has been queued for XDP TX we have no guarantee we will see
another interrupt, so run the reclaim there as well, to make sure
TX statistics won't become stale.

This should help us reclaim more buffers per single queue controller
register read.

Note that the XDP completion is very trivial, it only adds up
the sizes of transmitted frames for statistics so the latency
spike should be acceptable.  In case user sets the ring sizes
to something crazy, limit the completion to 2k entries.

The check if the ring is empty at the beginning of xdp_complete()
is no longer needed - the callers will perform it.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net.h       |  1 +
 .../net/ethernet/netronome/nfp/nfp_net_common.c    | 52 ++++++++++++++--------
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index acd9811d08d1..66319a1026bb 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -102,6 +102,7 @@
 #define NFP_NET_RX_DESCS_DEFAULT 4096	/* Default # of Rx descs per ring */
 
 #define NFP_NET_FL_BATCH	16	/* Add freelist in this Batch size */
+#define NFP_NET_XDP_MAX_COMPLETE 2048	/* XDP bufs to reclaim in NAPI poll */
 
 /* Offload definitions */
 #define NFP_NET_N_VXLAN_PORTS	(NFP_NET_CFG_VXLAN_SZ / sizeof(__be16))
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index d640b3331741..a4c6878f1df1 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -1001,27 +1001,30 @@ static void nfp_net_tx_complete(struct nfp_net_tx_ring *tx_ring)
 		  tx_ring->rd_p, tx_ring->wr_p, tx_ring->cnt);
 }
 
-static void nfp_net_xdp_complete(struct nfp_net_tx_ring *tx_ring)
+static bool nfp_net_xdp_complete(struct nfp_net_tx_ring *tx_ring)
 {
 	struct nfp_net_r_vector *r_vec = tx_ring->r_vec;
 	u32 done_pkts = 0, done_bytes = 0;
+	bool done_all;
 	int idx, todo;
 	u32 qcp_rd_p;
 
-	if (tx_ring->wr_p == tx_ring->rd_p)
-		return;
-
 	/* Work out how many descriptors have been transmitted */
 	qcp_rd_p = nfp_qcp_rd_ptr_read(tx_ring->qcp_q);
 
 	if (qcp_rd_p == tx_ring->qcp_rd_p)
-		return;
+		return true;
 
 	if (qcp_rd_p > tx_ring->qcp_rd_p)
 		todo = qcp_rd_p - tx_ring->qcp_rd_p;
 	else
 		todo = qcp_rd_p + tx_ring->cnt - tx_ring->qcp_rd_p;
 
+	done_all = todo <= NFP_NET_XDP_MAX_COMPLETE;
+	todo = min(todo, NFP_NET_XDP_MAX_COMPLETE);
+
+	tx_ring->qcp_rd_p = (tx_ring->qcp_rd_p + todo) & (tx_ring->cnt - 1);
+
 	done_pkts = todo;
 	while (todo--) {
 		idx = tx_ring->rd_p & (tx_ring->cnt - 1);
@@ -1030,16 +1033,16 @@ static void nfp_net_xdp_complete(struct nfp_net_tx_ring *tx_ring)
 		done_bytes += tx_ring->txbufs[idx].real_len;
 	}
 
-	tx_ring->qcp_rd_p = qcp_rd_p;
-
 	u64_stats_update_begin(&r_vec->tx_sync);
 	r_vec->tx_bytes += done_bytes;
 	r_vec->tx_pkts += done_pkts;
 	u64_stats_update_end(&r_vec->tx_sync);
 
 	WARN_ONCE(tx_ring->wr_p - tx_ring->rd_p > tx_ring->cnt,
-		  "TX ring corruption rd_p=%u wr_p=%u cnt=%u\n",
+		  "XDP TX ring corruption rd_p=%u wr_p=%u cnt=%u\n",
 		  tx_ring->rd_p, tx_ring->wr_p, tx_ring->cnt);
+
+	return done_all;
 }
 
 /**
@@ -1500,15 +1503,23 @@ static bool
 nfp_net_tx_xdp_buf(struct nfp_net_dp *dp, struct nfp_net_rx_ring *rx_ring,
 		   struct nfp_net_tx_ring *tx_ring,
 		   struct nfp_net_rx_buf *rxbuf, unsigned int dma_off,
-		   unsigned int pkt_len)
+		   unsigned int pkt_len, bool *completed)
 {
 	struct nfp_net_tx_buf *txbuf;
 	struct nfp_net_tx_desc *txd;
 	int wr_idx;
 
 	if (unlikely(nfp_net_tx_full(tx_ring, 1))) {
-		nfp_net_rx_drop(dp, rx_ring->r_vec, rx_ring, rxbuf, NULL);
-		return false;
+		if (!*completed) {
+			nfp_net_xdp_complete(tx_ring);
+			*completed = true;
+		}
+
+		if (unlikely(nfp_net_tx_full(tx_ring, 1))) {
+			nfp_net_rx_drop(dp, rx_ring->r_vec, rx_ring, rxbuf,
+					NULL);
+			return false;
+		}
 	}
 
 	wr_idx = tx_ring->wr_p & (tx_ring->cnt - 1);
@@ -1580,6 +1591,7 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget)
 	struct nfp_net_dp *dp = &r_vec->nfp_net->dp;
 	struct nfp_net_tx_ring *tx_ring;
 	struct bpf_prog *xdp_prog;
+	bool xdp_tx_cmpl = false;
 	unsigned int true_bufsz;
 	struct sk_buff *skb;
 	int pkts_polled = 0;
@@ -1690,7 +1702,8 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget)
 				if (unlikely(!nfp_net_tx_xdp_buf(dp, rx_ring,
 								 tx_ring, rxbuf,
 								 dma_off,
-								 pkt_len)))
+								 pkt_len,
+								 &xdp_tx_cmpl)))
 					trace_xdp_exception(dp->netdev,
 							    xdp_prog, act);
 				continue;
@@ -1738,8 +1751,14 @@ static int nfp_net_rx(struct nfp_net_rx_ring *rx_ring, int budget)
 		napi_gro_receive(&rx_ring->r_vec->napi, skb);
 	}
 
-	if (xdp_prog && tx_ring->wr_ptr_add)
-		nfp_net_tx_xmit_more_flush(tx_ring);
+	if (xdp_prog) {
+		if (tx_ring->wr_ptr_add)
+			nfp_net_tx_xmit_more_flush(tx_ring);
+		else if (unlikely(tx_ring->wr_p != tx_ring->rd_p) &&
+			 !xdp_tx_cmpl)
+			if (!nfp_net_xdp_complete(tx_ring))
+				pkts_polled = budget;
+	}
 	rcu_read_unlock();
 
 	return pkts_polled;
@@ -1760,11 +1779,8 @@ static int nfp_net_poll(struct napi_struct *napi, int budget)
 
 	if (r_vec->tx_ring)
 		nfp_net_tx_complete(r_vec->tx_ring);
-	if (r_vec->rx_ring) {
+	if (r_vec->rx_ring)
 		pkts_polled = nfp_net_rx(r_vec->rx_ring, budget);
-		if (r_vec->xdp_ring)
-			nfp_net_xdp_complete(r_vec->xdp_ring);
-	}
 
 	if (pkts_polled < budget)
 		if (napi_complete_done(napi, pkts_polled))
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 2/9] nfp: rename l4_offset in struct nfp_net_tx_desc to lso_hdrlen
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Edwin Peer
In-Reply-To: <20170516005523.26124-1-jakub.kicinski@netronome.com>

From: Edwin Peer <edwin.peer@netronome.com>

The l4_offset field referred to by NFD is confusingly named. It is not the
offset of the L4 transport header, but rather the L4 payload.

The LSO2 capability supported by alternative device firmware requires
the actual L4 offset, thus the rename seems prudent.

Signed-off-by: Edwin Peer <edwin.peer@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net.h        | 2 +-
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index fcf81b3be830..6bad11e5b845 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -153,7 +153,7 @@ struct nfp_net_tx_desc {
 			__le32 dma_addr_lo; /* Low 32bit of host buf addr */
 
 			__le16 mss;	/* MSS to be used for LSO */
-			u8 l4_offset;	/* LSO, where the L4 data starts */
+			u8 lso_hdrlen;	/* LSO, TCP payload offset */
 			u8 flags;	/* TX Flags, see @PCIE_DESC_TX_* */
 
 			__le16 vlan;	/* VLAN tag to add if indicated */
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 76251a09a1f3..0cebe9098451 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -677,7 +677,7 @@ static void nfp_net_tx_tso(struct nfp_net_r_vector *r_vec,
 	txbuf->real_len += hdrlen * (txbuf->pkt_cnt - 1);
 
 	mss = skb_shinfo(skb)->gso_size & PCIE_DESC_TX_MSS_MASK;
-	txd->l4_offset = hdrlen;
+	txd->lso_hdrlen = hdrlen;
 	txd->mss = cpu_to_le16(mss);
 	txd->flags |= PCIE_DESC_TX_LSO;
 
@@ -823,7 +823,7 @@ static int nfp_net_tx(struct sk_buff *skb, struct net_device *netdev)
 
 	txd->flags = 0;
 	txd->mss = 0;
-	txd->l4_offset = 0;
+	txd->lso_hdrlen = 0;
 
 	nfp_net_tx_tso(r_vec, txbuf, txd, skb);
 
@@ -1515,7 +1515,7 @@ nfp_net_tx_xdp_buf(struct nfp_net_dp *dp, struct nfp_net_rx_ring *rx_ring,
 
 	txd->flags = 0;
 	txd->mss = 0;
-	txd->l4_offset = 0;
+	txd->lso_hdrlen = 0;
 
 	tx_ring->wr_p++;
 	tx_ring->wr_ptr_add++;
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 1/9] nfp: don't enable TSO on the device when disabled
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski
In-Reply-To: <20170516005523.26124-1-jakub.kicinski@netronome.com>

We advertise TSO to the stack but leave it disabled by default.
Make sure it's not only disabled in the netdev features but
also on the device itself.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 82bd6b0935f1..76251a09a1f3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3286,6 +3286,7 @@ int nfp_net_netdev_init(struct net_device *netdev)
 
 	/* Advertise but disable TSO by default. */
 	netdev->features &= ~(NETIF_F_TSO | NETIF_F_TSO6);
+	nn->dp.ctrl &= ~NFP_NET_CFG_CTRL_LSO;
 
 	/* Allow L2 Broadcast and Multicast through by default, if supported */
 	if (nn->cap & NFP_NET_CFG_CTRL_L2BC)
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 0/9] nfp: LSO, checksum and XDP datapath updates
From: Jakub Kicinski @ 2017-05-16  0:55 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski

Hi!

This series introduces a number of refinements to standard features
like LSO and checksum offload.  Three major features are support for
CHECKSUM_COMPLETE, refinement of TSO handling and another small speed
up for XDP TX.  This series also switches from depending on some 
app FW<>driver ABI versions to heavier use of capabilities.


Edwin Peer (3):
  nfp: rename l4_offset in struct nfp_net_tx_desc to lso_hdrlen
  nfp: support LSO2 capability
  nfp: version independent support for chained RSS metadata

Jakub Kicinski (6):
  nfp: don't enable TSO on the device when disabled
  nfp: don't assume RSS and IRQ moderation are always enabled
  nfp: add CHECKSUM_COMPLETE support
  nfp: complete the XDP TX ring only when it's full
  nfp: add a helper for wrapping descriptor index
  nfp: eliminate an if statement in calculation of completed frames

 drivers/net/ethernet/netronome/nfp/nfp_net.h       |  19 ++-
 .../net/ethernet/netronome/nfp/nfp_net_common.c    | 179 +++++++++++++--------
 drivers/net/ethernet/netronome/nfp/nfp_net_ctrl.h  |  18 ++-
 .../net/ethernet/netronome/nfp/nfp_net_ethtool.c   |  12 +-
 4 files changed, 149 insertions(+), 79 deletions(-)

-- 
2.11.0

^ permalink raw reply

* Re: [PATCH] net: ethernet: broadcom: bnxt: remove dead code
From: Michael Chan @ 2017-05-16  0:34 UTC (permalink / raw)
  To: Gustavo A. R. Silva; +Cc: Netdev, open list
In-Reply-To: <20170515222818.GA17519@embeddedgus>

On Mon, May 15, 2017 at 3:28 PM, Gustavo A. R. Silva
<garsilva@embeddedor.com> wrote:
> Local variable _sh_ is assigned to a constant value and it is never updated
> again. Remove this variable and the dead code it guards.
>
> Addresses-Coverity-ID: 1350916
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 1f1e54b..018674b 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -7380,12 +7380,10 @@ static int bnxt_get_dflt_rings(struct bnxt *bp, int *max_rx, int *max_tx,
>  static int bnxt_set_dflt_rings(struct bnxt *bp)
>  {
>         int dflt_rings, max_rx_rings, max_tx_rings, rc;
> -       bool sh = true;

The point of this logic is that we can easily change the default to
shared rings or separate rings.  I think what I'll do is change it so
that the parameter sh is passed in and let the caller decide.  Thanks
for pointing this out.

>
> -       if (sh)
> -               bp->flags |= BNXT_FLAG_SHARED_RINGS;
> +       bp->flags |= BNXT_FLAG_SHARED_RINGS;
>         dflt_rings = netif_get_num_default_rss_queues();
> -       rc = bnxt_get_dflt_rings(bp, &max_rx_rings, &max_tx_rings, sh);
> +       rc = bnxt_get_dflt_rings(bp, &max_rx_rings, &max_tx_rings, true);
>         if (rc)
>                 return rc;
>         bp->rx_nr_rings = min_t(int, dflt_rings, max_rx_rings);
> @@ -7396,8 +7394,7 @@ static int bnxt_set_dflt_rings(struct bnxt *bp)
>                 netdev_warn(bp->dev, "Unable to reserve tx rings\n");
>
>         bp->tx_nr_rings = bp->tx_nr_rings_per_tc;
> -       bp->cp_nr_rings = sh ? max_t(int, bp->tx_nr_rings, bp->rx_nr_rings) :
> -                              bp->tx_nr_rings + bp->rx_nr_rings;
> +       bp->cp_nr_rings = max_t(int, bp->tx_nr_rings, bp->rx_nr_rings);
>         bp->num_stat_ctxs = bp->cp_nr_rings;
>         if (BNXT_CHIP_TYPE_NITRO_A0(bp)) {
>                 bp->rx_nr_rings++;
> --
> 2.5.0
>

^ permalink raw reply

* (unknown), 
From: morice.diane @ 2017-05-15 23:49 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: EMAIL_0461021_netdev.zip --]
[-- Type: application/zip, Size: 2062 bytes --]

^ permalink raw reply

* (unknown), 
From: bcohen @ 2017-05-15 23:19 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: EMAIL_94874074783512_netdev.zip --]
[-- Type: application/zip, Size: 2074 bytes --]

^ permalink raw reply

* Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
From: Luis R. Rodriguez @ 2017-05-15 23:13 UTC (permalink / raw)
  To: Arend Van Spriel, Johannes Berg
  Cc: Luis R. Rodriguez, Pavel Machek, Daniel Wagner, Tom Gundersen,
	Pali Rohár, Greg Kroah-Hartman, Kalle Valo, David Gnedt,
	Tony Lindgren, Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
	Takashi Iwai, AKASHI Takahiro, David Woodhouse, Bjorn Andersson,
	Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <0fd90416-f33c-a6be-14fd-5e964583e9cb@broadcom.com>

On Fri, May 12, 2017 at 11:02:26PM +0200, Arend Van Spriel wrote:
> try again.. replacing email address from Michał
> On 12-5-2017 22:55, Arend Van Spriel wrote:
> > Let me explain the idea to refresh your memory (and mine). It started
> > when we were working on adding driver support for OpenWrt in brcmfmac.
> > The driver requests for firmware calibration data, but on routers it is
> > stored in flash. So after failing on the firmware request we now call a
> > platform specific API. That was my itch, but it was not bad enough to go
> > and scratch. Now for N900 case there is a similar scenario alhtough it
> > has additional requirement to go to user-space due to need to use a
> > proprietary library to obtain the NVS calibration data. My thought: Why
> > should firmware_class care?

Agreed.

> > So the idea is that firmware_class provides
> > a registry for modules that can produce a certain firmware "file". Those
> > modules can do whatever is needed. If they need to use umh so be it.
> > They would only register themselves with firmware_class on platforms
> > that need them. It would basically be replacing the fallback mechanism
> > and only be effective on certain platforms.

Sure, so it sounds like the work that Daniel Wagner and Tom Gundersen worked
[0] on which provides a firmwared with two modes: best-effort, and final-mode,
would address what you are looking for but without requiring any upstream
changes, *and* it also helps solve the rootfs race remote-proc folks had
concerns over.

The other added gain over this solution is if folks need their own proprietary
concoction they can just fork firmwared and have that do whatever it needs
for the specific device on the specific rootfs. That is, firmwared can be the
upstream solution if folks need it, but if folks need something custom they can
just mimic the implementation: best-effort, and and final-mode.

Yet another added gain over this solution we can do *not* support the
custom fallback mechanism as its not needed, the udev event should suffice
to let userspace do what it needs.

Lastly, if we did not want to deal with timeouts for the way the driver data
API implements it I think we might be able to do away with them for for async
requests if we assume there will be a daemon that spawns in final-mode eventually,
and since it *knows* when the rootfs is ready it should be able to do a final
lookup, if it returns -ENOENT; then indeed we know we can give up. Now, perhaps
how and if we want to deal with timeouts when using the driver data API for
the fallback mechanism is worth considering given it does not have a fallback
mechanism support yet. If we *add* them it would seem this would also put an
implicit race against userspace finishing initialization and running firmwared
in final-mode.

Johannes, do you recall the corner cases we spoke about regarding timeouts?
Does this match what we spoke about?

> > Let me know if this idea is still of interest and I will rebase what I
> > have for an RFC round.

Since no upstream delta is needed for firmwared I'd like to first encourage
evaluating the above. While distributions don't carry it yet that may be seen as
an issue but since what we are looking for are corner cases, only folks needing
to deploy a specific solution would need it or a custom proprietary solution.

[0] https://github.com/teg/firmwared.git

PS.

Note that firmware signing will require an additional file, the detached
signature. The driver data API does not currently support the fallback
mechanism so we would not have to worry about that yet but once we add
fallback support we'd need to consider this.

  Luis

^ permalink raw reply

* Re: [net 1/6] net/mlx5e: Use a spinlock to synchronize statistics
From: Stephen Hemminger @ 2017-05-15 22:51 UTC (permalink / raw)
  To: Saeed Mahameed; +Cc: David S. Miller, netdev, Gal Pressman, kernel-team
In-Reply-To: <20170512115650.11635-2-saeedm@mellanox.com>

On Fri, 12 May 2017 14:56:45 +0300
Saeed Mahameed <saeedm@mellanox.com> wrote:

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 0099a3e397bc..c41cf7e82795 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -756,6 +756,7 @@ struct mlx5e_priv {
>  	struct mlx5_core_dev      *mdev;
>  	struct net_device         *netdev;
>  	struct mlx5e_stats         stats;
> +	rwlock_t                   stats_lock;

A reader-writer lock is slower than a spin_lock.
The only case where reader/writer lock is preferred is when writer
will hold onto the lock for a significant amount of time.
Also you only need each statistic to be atomic, the network device API
never guarantees transactional update across multiple statistics.

^ permalink raw reply

* BUG: cxgb4 is broken in v4.12-rc1
From: Logan Gunthorpe @ 2017-05-15 22:36 UTC (permalink / raw)
  To: Ganesh Goudar; +Cc: David S. Miller, Stephen Bates, Steve Wise, netdev

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

Hi,

With rc1 my T62100-LP-CR no longer functions correctly. Everything
appears fine but the link never goes into the UP state. I have one peer
with an older (functioning) kernel and the other peer on rc1.

I've bisected to find this is the offending commit:

3bb4858fd: cxgb4: avoid disabling FEC by default

I've also attached a bisect log.

Let me know if you need anything else.

Thanks,

Logan

[-- Attachment #2: bisect.log --]
[-- Type: text/x-log, Size: 2644 bytes --]

git bisect start
# bad: [2ea659a9ef488125eb46da6eb571de5eae5c43f6] Linux 4.12-rc1
git bisect bad 2ea659a9ef488125eb46da6eb571de5eae5c43f6
# good: [c02ed2e75ef4c74e41e421acb4ef1494671585e8] Linux 4.11-rc4
git bisect good c02ed2e75ef4c74e41e421acb4ef1494671585e8
# good: [2f34c1231bfc9f2550f934acb268ac7315fb3837] Merge tag 'drm-for-v4.12' of git://people.freedesktop.org/~airlied/linux
git bisect good 2f34c1231bfc9f2550f934acb268ac7315fb3837
# good: [c6a677c6f37bb7abc85ba7e3465e82b9f7eb1d91] Merge tag 'staging-4.12-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
git bisect good c6a677c6f37bb7abc85ba7e3465e82b9f7eb1d91
# good: [0ff4c01b279a590a2826ade9321ad8c7ca5a1b6c] Merge tag 'armsoc-arm64' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect good 0ff4c01b279a590a2826ade9321ad8c7ca5a1b6c
# bad: [c70422f760c120480fee4de6c38804c72aa26bc1] Merge tag 'nfsd-4.12' of git://linux-nfs.org/~bfields/linux
git bisect bad c70422f760c120480fee4de6c38804c72aa26bc1
# bad: [56868a460b83c0f93d339256a81064d89aadae8e] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide
git bisect bad 56868a460b83c0f93d339256a81064d89aadae8e
# good: [ae706bebd6a172a6adf9f63d1a77b8ed49e998a6] Merge tag 'juno-fixes-4.12' of git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux into next/dt64
git bisect good ae706bebd6a172a6adf9f63d1a77b8ed49e998a6
# good: [ecc721a72c121e8b641d68efd24a225abedb9a30] Merge tag 'pwm/for-4.12-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm
git bisect good ecc721a72c121e8b641d68efd24a225abedb9a30
# bad: [657831ffc38e30092a2d5f03d385d710eb88b09a] dccp/tcp: do not inherit mc_list from parent
git bisect bad 657831ffc38e30092a2d5f03d385d710eb88b09a
# bad: [b210aeaedfc47ab8a86fbdcad46caf1e49cc0e37] Merge branch 'stmmac-pci-fix-crash-on-Intel-Galileo-Gen2'
git bisect bad b210aeaedfc47ab8a86fbdcad46caf1e49cc0e37
# good: [ac45bd93a5035c2f39c9862b8b6ed692db0fdc87] bnxt_en: allocate enough space for ->ntp_fltr_bmap
git bisect good ac45bd93a5035c2f39c9862b8b6ed692db0fdc87
# bad: [0d0e57697f162da4aa218b5feafe614fb666db07] bpf: don't let ldimm64 leak map addresses on unprivileged
git bisect bad 0d0e57697f162da4aa218b5feafe614fb666db07
# bad: [3bb4858fda1cac2ae677edb5134932af4c7f8ff8] cxgb4: avoid disabling FEC by default
git bisect bad 3bb4858fda1cac2ae677edb5134932af4c7f8ff8
# good: [8ce7aaaa9797f944ad2f50e5160c8a20a473aedf] net: dsa: loop: Check for memory allocation failure
git bisect good 8ce7aaaa9797f944ad2f50e5160c8a20a473aedf
# first bad commit: [3bb4858fda1cac2ae677edb5134932af4c7f8ff8] cxgb4: avoid disabling FEC by default

^ permalink raw reply

* Re: [PATCH] neighbour: update neigh timestamps iff update is effective
From: Julian Anastasov @ 2017-05-15 22:27 UTC (permalink / raw)
  To: Ihar Hrachyshka; +Cc: David S. Miller, He Chunhui, netdev
In-Reply-To: <CAKwN9=C-_F-8GQ7yMO6wVR1Ukw3gCBYOzxxDRRLU9b-u9P1v-Q@mail.gmail.com>


	Hello,

On Mon, 15 May 2017, Ihar Hrachyshka wrote:

> On Mon, May 15, 2017 at 1:05 PM, Julian Anastasov <ja@ssi.bg> wrote:
> >
> >         It seems arp_accept value currently has influence on
> > the locktime for GARP requests. My understanding is that
> > locktime is used to ignore replies from proxy_arp
> > routers while the requested IP is present on the LAN
> > and replies immediately. IMHO, GARP requests should not
> > depend on locktime, even when arp_accept=0. For example:
> 
> Yes, I believe so.
> 
> I actually thought about introducing the patch that does just that:
> forcing override on garp, but then I was thinking, maybe there is some
> reason to still apply locktime rules to garps; f.e. if you have
> multiple nodes carrying the ip address and located on the same
> segment, maybe you want to pick the first that replies to you (in
> theory, it may be the node that is less loaded, or closer to us; but
> then, it's so fragile even if that was the intent...) Do you want me
> to post the patch, or will you cover it?

	Feel free to post a patch for this, I see that you change
in another patch the is_garp value, so it seems the same logic
should be used twice.

Regards

^ permalink raw reply

* [PATCH] net: ethernet: broadcom: bnxt: remove dead code
From: Gustavo A. R. Silva @ 2017-05-15 22:28 UTC (permalink / raw)
  To: Michael Chan; +Cc: netdev, linux-kernel, Gustavo A. R. Silva

Local variable _sh_ is assigned to a constant value and it is never updated
again. Remove this variable and the dead code it guards.

Addresses-Coverity-ID: 1350916
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 1f1e54b..018674b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7380,12 +7380,10 @@ static int bnxt_get_dflt_rings(struct bnxt *bp, int *max_rx, int *max_tx,
 static int bnxt_set_dflt_rings(struct bnxt *bp)
 {
 	int dflt_rings, max_rx_rings, max_tx_rings, rc;
-	bool sh = true;
 
-	if (sh)
-		bp->flags |= BNXT_FLAG_SHARED_RINGS;
+	bp->flags |= BNXT_FLAG_SHARED_RINGS;
 	dflt_rings = netif_get_num_default_rss_queues();
-	rc = bnxt_get_dflt_rings(bp, &max_rx_rings, &max_tx_rings, sh);
+	rc = bnxt_get_dflt_rings(bp, &max_rx_rings, &max_tx_rings, true);
 	if (rc)
 		return rc;
 	bp->rx_nr_rings = min_t(int, dflt_rings, max_rx_rings);
@@ -7396,8 +7394,7 @@ static int bnxt_set_dflt_rings(struct bnxt *bp)
 		netdev_warn(bp->dev, "Unable to reserve tx rings\n");
 
 	bp->tx_nr_rings = bp->tx_nr_rings_per_tc;
-	bp->cp_nr_rings = sh ? max_t(int, bp->tx_nr_rings, bp->rx_nr_rings) :
-			       bp->tx_nr_rings + bp->rx_nr_rings;
+	bp->cp_nr_rings = max_t(int, bp->tx_nr_rings, bp->rx_nr_rings);
 	bp->num_stat_ctxs = bp->cp_nr_rings;
 	if (BNXT_CHIP_TYPE_NITRO_A0(bp)) {
 		bp->rx_nr_rings++;
-- 
2.5.0

^ permalink raw reply related

* [PATCH] ipmr: vrf: Find VIFs using the actual device
From: Thomas Winter @ 2017-05-15 22:14 UTC (permalink / raw)
  To: netdev; +Cc: Thomas Winter, David Ahern, Nikolay Aleksandrov, roopa

The skb->dev that is passed into ip_mr_input is
the loX device for VRFs. When we lookup a vif
for this dev, none is found as we do not create
vifs for loopbacks. Instead lookup a vif for the
actual device that the packet was received on,
eg the vlan.

Signed-off-by: Thomas Winter <Thomas.Winter@alliedtelesis.co.nz>
cc: David Ahern <dsa@cumulusnetworks.com>
cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
cc: roopa <roopa@cumulusnetworks.com>
---
 net/ipv4/ipmr.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 3a02d52ed50e..551de4d023a8 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1980,6 +1980,20 @@ int ip_mr_input(struct sk_buff *skb)
 	struct net *net = dev_net(skb->dev);
 	int local = skb_rtable(skb)->rt_flags & RTCF_LOCAL;
 	struct mr_table *mrt;
+	struct net_device *dev;
+
+	/* skb->dev passed in is the loX master dev for vrfs.
+	 * As there are no vifs associated with loopback devices,
+	 * get the proper interface that does have a vif associated with it.
+	 */
+	dev = skb->dev;
+	if (netif_is_l3_master(skb->dev)) {
+		dev = dev_get_by_index_rcu(net, IPCB(skb)->iif);
+		if (!dev) {
+			kfree_skb(skb);
+			return -ENODEV;
+		}
+	}
 
 	/* Packet is looped back after forward, it should not be
 	 * forwarded second time, but still can be delivered locally.
@@ -2017,7 +2031,7 @@ int ip_mr_input(struct sk_buff *skb)
 	/* already under rcu_read_lock() */
 	cache = ipmr_cache_find(mrt, ip_hdr(skb)->saddr, ip_hdr(skb)->daddr);
 	if (!cache) {
-		int vif = ipmr_find_vif(mrt, skb->dev);
+		int vif = ipmr_find_vif(mrt, dev);
 
 		if (vif >= 0)
 			cache = ipmr_cache_find_any(mrt, ip_hdr(skb)->daddr,
@@ -2037,7 +2051,7 @@ int ip_mr_input(struct sk_buff *skb)
 		}
 
 		read_lock(&mrt_lock);
-		vif = ipmr_find_vif(mrt, skb->dev);
+		vif = ipmr_find_vif(mrt, dev);
 		if (vif >= 0) {
 			int err2 = ipmr_cache_unresolved(mrt, vif, skb);
 			read_unlock(&mrt_lock);
-- 
2.11.0

^ permalink raw reply related

* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: Cong Wang @ 2017-05-15 22:13 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: Eric Dumazet, David Miller, Linux Kernel Network Developers,
	Andrey Konovalov, Eric Dumazet
In-Reply-To: <alpine.LFD.2.20.1705152317090.4002@ja.home.ssi.bg>

On Mon, May 15, 2017 at 1:37 PM, Julian Anastasov <ja@ssi.bg> wrote:
>         Any user that does not set FIB_LOOKUP_NOREF
> will need nh_dev refcounts. The assumption is that the
> NHs are accessed, who knows, may be even after RCU grace
> period. As result, we can not use dev_put on NETDEV_UNREGISTER.
> So, we should check if there are users that do not
> set FIB_LOOKUP_NOREF, at first look, I don't see such ones
> for IPv4.

I see, although we do have FIB_LOOKUP_NOREF set all the times,
there are other places we hold fib_clntref too, for example
mlxsw_sp_router_fib_event_work(), it actually uses nh_dev too...

So I am afraid moving dev_put() to fib_release_info() is not a solution
here. I have to rethink about it.

^ permalink raw reply

* Re: [PATCH iproute2 -master 0/2] Two misc BPF updates
From: Stephen Hemminger @ 2017-05-15 22:07 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: alexei.starovoitov, netdev
In-Reply-To: <cover.1494635217.git.daniel@iogearbox.net>

On Sat, 13 May 2017 02:32:33 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> Requires header rebase with -net.
> 
> Thanks!
> 
> Daniel Borkmann (2):
>   bpf: update printing of generic xdp mode
>   bpf: dump error to the user when retrieving pinned prog fails
> 
>  ip/iplink_xdp.c | 19 +++++++++++--------
>  lib/bpf.c       | 12 +++++++++++-
>  2 files changed, 22 insertions(+), 9 deletions(-)
> 

Holding off until Linus pulls net

^ permalink raw reply

* Re: [PATCH iproute2 master 0/4] pedit: Introduce IPv6 support + some minor fixes
From: Stephen Hemminger @ 2017-05-15 22:06 UTC (permalink / raw)
  To: Amir Vadai; +Cc: netdev, Or Gerlitz
In-Reply-To: <20170514081746.9010-1-amir@vadai.me>

On Sun, 14 May 2017 11:17:42 +0300
Amir Vadai <amir@vadai.me> wrote:

> Hi,
> 
> This patchset introduces pedit IPv6 support.
> Almost all IPv6 header fields are editable now (src, dst, flow_lbl,
> payload_len, next_hdr and hoplimit).
> The patch uses the new extended pedit netlink and will fail the operation if
> kernel has no support or user didn't use the 'ex' keyword.
> In addition to this patch, 3 more patches fix some minor UI issues:
> - some typo's
> - 'retain' can't be used with fields > 32 bits. It will make unexpected things
> 	when used in such fields. Fixing this limitiation requires some changes (in
> 	tc user space only) that are out of the scope of this patchset. So I added a
> 	patch to prevent the user from using retain on those fields.
> 
> 
> Thanks,
> Amir	
> 
> Amir Vadai (4):
>   pedit: Fix a typo in warning
>   pedit: Do not allow using retain for too big fields
>   pedit: Check for extended capability in protocol parser
>   pedit: Introduce ipv6 support
> 
>  man/man8/tc-pedit.8 | 33 ++++++++++++++++++-
>  tc/Makefile         |  1 +
>  tc/m_pedit.c        | 51 ++++++++++++++++++++++++++++--
>  tc/p_eth.c          |  3 ++
>  tc/p_ip.c           | 17 +---------
>  tc/p_ip6.c          | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tc/p_udp.c          |  3 ++
>  7 files changed, 179 insertions(+), 20 deletions(-)
>  create mode 100644 tc/p_ip6.c
> 

Looks good. Applied.

^ permalink raw reply

* Re: [PATCH 1/1] dt-binding: net: wireless: fix node name in the BCM43xx example
From: Arend Van Spriel @ 2017-05-15 22:05 UTC (permalink / raw)
  To: Martin Blumenstingl, kvalo-sgV2jX0FEOL9JmXXK+q4OQ,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170515201356.26384-2-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On 15-5-2017 22:13, Martin Blumenstingl wrote:
> The example in the BCM43xx documentation uses "brcmf" as node name.
> However, wireless devices should be named "wifi" instead. Fix this to

Hi Martin,

Since when is that a rule. I never got the memo and the DTC did not ever
complain to me about the naming. That being said I do not really care
and I suppose it is for the sake of consistency only.

> make sure that .dts authors can simply use the documentation as
> reference (or simply copy the node from the documentation and then
> adjust only the board specific bits).

Please feel free to add my...

Acked-by: Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> index 5dbf169cd81c..590f622188de 100644
> --- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm43xx-fmac.txt
> @@ -31,7 +31,7 @@ mmc3: mmc@01c12000 {
>  	non-removable;
>  	status = "okay";
>  
> -	brcmf: bcrmf@1 {
> +	brcmf: wifi@1 {
>  		reg = <1>;
>  		compatible = "brcm,bcm4329-fmac";
>  		interrupt-parent = <&pio>;
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/3] bpf: Track alignment of MAP pointers in verifier.
From: Daniel Borkmann @ 2017-05-15 21:55 UTC (permalink / raw)
  To: David Miller; +Cc: ast, alexei.starovoitov, netdev
In-Reply-To: <20170515.113419.1265777991542814689.davem@davemloft.net>

On 05/15/2017 05:34 PM, David Miller wrote:
> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Mon, 15 May 2017 15:10:02 +0200
>
>>>> What are the semantics of using id here? In ptr_to_pkt, we have it,
>>>> so that eventually, in find_good_pkt_pointers() we can match on id
>>>> and update the range for all such regs with the same id. I'm just
>>>> wondering as the side effect of this is that this makes state
>>>> pruning worse.
>
> Daniel, I looked at the state pruning for maps.  The situation is
> quite interesting.
>
> Once we have env->varlen_map_value_access (and load or store via a
> PTR_TO_MAP_VALUE_ADJ pointer), the state pruning gets more strict, the
> relevant tests are:
>
> 		if (memcmp(rold, rcur, sizeof(*rold)) == 0)
> 			continue;
>
> 		/* If the ranges were not the same, but everything else was and
> 		 * we didn't do a variable access into a map then we are a-ok.
> 		 */
> 		if (!varlen_map_access &&
> 		    memcmp(rold, rcur, offsetofend(struct bpf_reg_state, id)) == 0)
> 			continue;
>
> The first memcmp() is not going to match any time we adjust any
> component of a MAP pointer reg.  The offset, the alignment, anything.
> That means any side effect whatsoever performed by check_pointer_add()
> even if we changed the code to not modify reg->id
>
> The second check elides:
>
> 	s64 min_value;
> 	u64 max_value;
> 	u32 min_align;
> 	u32 aux_off;
> 	u32 aux_off_align;
>
> from the comparison but only if we haven't done a variable length
> MAP access.

I'm actually wondering about the min_align/aux_off/aux_off_align and
given this is not really related to varlen_map_access and we currently
just skip this.

We should make sure that when env->strict_alignment is false that we
ignore any difference in min_align/aux_off/aux_off_align, afaik, the
min_align would also be set on regs other than ptr_to_pkt.

What about compare_ptrs_to_packet() for when env->strict_alignment is
true in ptr_to_pkt case? Could we have a situation that prunes the search
with matching the third test? Say, in the old case, we did go all the
way and ...

   R3(off=0, r=0)
   R4 = R3 + 20   // AAA
   // now R4(off=20,r=0)
   if (R4 > data_end)
       got out;
   // BBB: now R4(off=20,r=20), R3(off=0,r=20) and R3 used to access

... verify the code under 'BBB' and found that it's safe to run,
including alignment, etc. Next time we come to this branch through
R4 = R3 + 33 (under AAA), so we have R4(off=33,r=0). What happens
if we then do R4-=4, and access 4 bytes of the packet?

The old R4(off=20,r=20) becomes R4(off=16,r=20), which was found
safe and the new R4(off=33,r=0) becomes R4(off=29,r=33) which
would end up being unaligned? Looks like we shouldn't prune in
such case? Maybe test_verifier test case helps to visualize.

> The only conclusion I can come to is that changing reg->id for
> PTR_TO_MAP_VALUE_ADJ has no side effect for state pruning, unless we
> perform PTR_TO_MAP_VALUE_ADJ register adjustments without ever
> accessing the map via that pointer in the entire program.

Why entire program, just between two state pruning points, no?
(They are marked as STATE_LIST_MARK.)

> I could add some new state to avoid the reg->id change, but given
> the above I don't think that it is really necessary.

^ permalink raw reply

* Re: [PATCH] neighbour: update neigh timestamps iff update is effective
From: Ihar Hrachyshka @ 2017-05-15 21:40 UTC (permalink / raw)
  To: davem; +Cc: ja, netdev, Ihar Hrachyshka
In-Reply-To: <20170515.131023.1878814687849300375.davem@davemloft.net>

It's a common practice to send gratuitous ARPs after moving an
IP address to another device to speed up healing of a service. To
fulfill service availability constraints, the timing of network peers
updating their caches to point to a new location of an IP address can be
particularly important.

Sometimes neigh_update calls won't touch neither lladdr nor state, for
example if an update arrives in locktime interval. The neigh->updated
value is tested by the protocol specific neigh code, which in turn
will influence whether NEIGH_UPDATE_F_OVERRIDE gets set in the
call to neigh_update() or not. As a result, we may effectively ignore
the update request, bailing out of touching the neigh entry, except that
we still bump its timestamps inside neigh_update.

This may be a problem for updates arriving in quick succession. For
example, consider the following scenario:

A service is moved to another device with its IP address. The new device
sends three gratuitous ARP requests into the network with ~1 seconds
interval between them. Just before the first request arrives to one of
network peer nodes, its neigh entry for the IP address transitions from
STALE to DELAY.  This transition, among other things, updates
neigh->updated. Once the kernel receives the first gratuitous ARP, it
ignores it because its arrival time is inside the locktime interval. The
kernel still bumps neigh->updated. Then the second gratuitous ARP
request arrives, and it's also ignored because it's still in the (new)
locktime interval. Same happens for the third request. The node
eventually heals itself (after delay_first_probe_time seconds since the
initial transition to DELAY state), but it just wasted some time and
require a new ARP request/reply round trip. This unfortunate behaviour
both puts more load on the network, as well as reduces service
availability.

This patch changes neigh_update so that it bumps neigh->updated (as well
as neigh->confirmed) only once we are sure that either lladdr or entry
state will change). In the scenario described above, it means that the
second gratuitous ARP request will actually update the entry lladdr.

Ideally, we would update the neigh entry on the very first gratuitous
ARP request. The locktime mechanism is designed to ignore ARP updates in
a short timeframe after a previous ARP update was honoured by the kernel
layer. This would require tracking timestamps for state transitions
separately from timestamps when actual updates are received. This would
probably involve changes in neighbour struct. Therefore, the patch
doesn't tackle the issue of the first gratuitous APR ignored, leaving
it for a follow-up.

Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
---
 net/core/neighbour.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 58b0bcc..d274f81 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1132,10 +1132,6 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 		lladdr = neigh->ha;
 	}
 
-	if (new & NUD_CONNECTED)
-		neigh->confirmed = jiffies;
-	neigh->updated = jiffies;
-
 	/* If entry was valid and address is not changed,
 	   do not change entry state, if new one is STALE.
 	 */
@@ -1157,6 +1153,16 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 		}
 	}
 
+	/* Update timestamps only once we know we will make a change to the
+	 * neighbour entry. Otherwise we risk to move the locktime window with
+	 * noop updates and ignore relevant ARP updates.
+	 */
+	if (new != old || lladdr != neigh->ha) {
+		if (new & NUD_CONNECTED)
+			neigh->confirmed = jiffies;
+		neigh->updated = jiffies;
+	}
+
 	if (new != old) {
 		neigh_del_timer(neigh);
 		if (new & NUD_PROBE)
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH] neighbour: update neigh timestamps iff update is effective
From: Ihar Hrachyshka @ 2017-05-15 21:35 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: David S. Miller, He Chunhui, netdev
In-Reply-To: <alpine.LFD.2.20.1705152234490.4002@ja.home.ssi.bg>

On Mon, May 15, 2017 at 1:05 PM, Julian Anastasov <ja@ssi.bg> wrote:
>
>         It seems arp_accept value currently has influence on
> the locktime for GARP requests. My understanding is that
> locktime is used to ignore replies from proxy_arp
> routers while the requested IP is present on the LAN
> and replies immediately. IMHO, GARP requests should not
> depend on locktime, even when arp_accept=0. For example:

Yes, I believe so.

I actually thought about introducing the patch that does just that:
forcing override on garp, but then I was thinking, maybe there is some
reason to still apply locktime rules to garps; f.e. if you have
multiple nodes carrying the ip address and located on the same
segment, maybe you want to pick the first that replies to you (in
theory, it may be the node that is less loaded, or closer to us; but
then, it's so fragile even if that was the intent...) Do you want me
to post the patch, or will you cover it?

Ihar

^ permalink raw reply


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