Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next resend 05/13] nfp: loosen relation between rings and IRQs vectors
From: Jakub Kicinski @ 2016-11-03 17:12 UTC (permalink / raw)
  To: netdev; +Cc: Jakub Kicinski
In-Reply-To: <1478193129-23476-1-git-send-email-jakub.kicinski@netronome.com>

Upcoming XDP support will break the assumption that one can iterate
over IRQ vectors to get to all the rings easily.  Use nn->.x_ring
arrays directly.

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

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 50aeaea9e318..97cc21eae466 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -1920,9 +1920,9 @@ static void nfp_net_clear_config_and_disable(struct nfp_net *nn)
 		nn_err(nn, "Could not disable device: %d\n", err);
 
 	for (r = 0; r < nn->num_rx_rings; r++)
-		nfp_net_rx_ring_reset(nn->r_vecs[r].rx_ring);
+		nfp_net_rx_ring_reset(&nn->rx_rings[r]);
 	for (r = 0; r < nn->num_tx_rings; r++)
-		nfp_net_tx_ring_reset(nn, nn->r_vecs[r].tx_ring);
+		nfp_net_tx_ring_reset(nn, &nn->tx_rings[r]);
 	for (r = 0; r < nn->num_r_vecs; r++)
 		nfp_net_vec_clear_ring_data(nn, r);
 
@@ -2000,7 +2000,7 @@ static int __nfp_net_set_config_and_enable(struct nfp_net *nn)
 	nn->ctrl = new_ctrl;
 
 	for (r = 0; r < nn->num_rx_rings; r++)
-		nfp_net_rx_ring_fill_freelist(nn->r_vecs[r].rx_ring);
+		nfp_net_rx_ring_fill_freelist(&nn->rx_rings[r]);
 
 	/* Since reconfiguration requests while NFP is down are ignored we
 	 * have to wipe the entire VXLAN configuration and reinitialize it.
@@ -2173,11 +2173,11 @@ static void nfp_net_close_free_all(struct nfp_net *nn)
 	unsigned int r;
 
 	for (r = 0; r < nn->num_rx_rings; r++) {
-		nfp_net_rx_ring_bufs_free(nn, nn->r_vecs[r].rx_ring);
-		nfp_net_rx_ring_free(nn->r_vecs[r].rx_ring);
+		nfp_net_rx_ring_bufs_free(nn, &nn->rx_rings[r]);
+		nfp_net_rx_ring_free(&nn->rx_rings[r]);
 	}
 	for (r = 0; r < nn->num_tx_rings; r++)
-		nfp_net_tx_ring_free(nn->r_vecs[r].tx_ring);
+		nfp_net_tx_ring_free(&nn->tx_rings[r]);
 	for (r = 0; r < nn->num_r_vecs; r++)
 		nfp_net_cleanup_vector(nn, &nn->r_vecs[r]);
 
-- 
1.9.1

^ permalink raw reply related
* [PATCH net-next resend 06/13] nfp: add helper to reassign rings to IRQ vectors
From: Jakub Kicinski @ 2016-11-03 17:12 UTC (permalink / raw)
  To: netdev; +Cc: Jakub Kicinski
In-Reply-To: <1478193129-23476-1-git-send-email-jakub.kicinski@netronome.com>

Instead of fixing ring -> vector relations up in ring swap functions
put the reassignment into a helper function which will reinit all
links.

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

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 97cc21eae466..2a4e1f1cb3c9 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -1601,16 +1601,11 @@ static int nfp_net_tx_ring_alloc(struct nfp_net_tx_ring *tx_ring, u32 cnt)
 static void
 nfp_net_tx_ring_set_swap(struct nfp_net *nn, struct nfp_net_ring_set *s)
 {
-	struct nfp_net_tx_ring *rings = s->rings;
 	struct nfp_net_ring_set new = *s;
-	unsigned int r;
 
 	s->dcnt = nn->txd_cnt;
 	s->rings = nn->tx_rings;
 
-	for (r = 0; r < nn->num_tx_rings; r++)
-		nn->tx_rings[r].r_vec->tx_ring = &rings[r];
-
 	nn->txd_cnt = new.dcnt;
 	nn->tx_rings = new.rings;
 }
@@ -1728,17 +1723,12 @@ static void nfp_net_rx_ring_free(struct nfp_net_rx_ring *rx_ring)
 static void
 nfp_net_rx_ring_set_swap(struct nfp_net *nn, struct nfp_net_ring_set *s)
 {
-	struct nfp_net_rx_ring *rings = s->rings;
 	struct nfp_net_ring_set new = *s;
-	unsigned int r;
 
 	s->mtu = nn->netdev->mtu;
 	s->dcnt = nn->rxd_cnt;
 	s->rings = nn->rx_rings;
 
-	for (r = 0; r < nn->num_rx_rings; r++)
-		nn->rx_rings[r].r_vec->rx_ring = &rings[r];
-
 	nn->netdev->mtu = new.mtu;
 	nn->fl_bufsz = nfp_net_calc_fl_bufsz(nn, new.mtu);
 	nn->rxd_cnt = new.dcnt;
@@ -1759,6 +1749,14 @@ static void nfp_net_rx_ring_free(struct nfp_net_rx_ring *rx_ring)
 	kfree(rings);
 }
 
+static void
+nfp_net_vector_assign_rings(struct nfp_net *nn, struct nfp_net_r_vector *r_vec,
+			    int idx)
+{
+	r_vec->rx_ring = idx < nn->num_rx_rings ? &nn->rx_rings[idx] : NULL;
+	r_vec->tx_ring = idx < nn->num_tx_rings ? &nn->tx_rings[idx] : NULL;
+}
+
 static int
 nfp_net_prepare_vector(struct nfp_net *nn, struct nfp_net_r_vector *r_vec,
 		       int idx)
@@ -1766,20 +1764,6 @@ static void nfp_net_rx_ring_free(struct nfp_net_rx_ring *rx_ring)
 	struct msix_entry *entry = &nn->irq_entries[r_vec->irq_idx];
 	int err;
 
-	if (idx < nn->num_tx_rings) {
-		r_vec->tx_ring = &nn->tx_rings[idx];
-		nfp_net_tx_ring_init(r_vec->tx_ring, r_vec, idx);
-	} else {
-		r_vec->tx_ring = NULL;
-	}
-
-	if (idx < nn->num_rx_rings) {
-		r_vec->rx_ring = &nn->rx_rings[idx];
-		nfp_net_rx_ring_init(r_vec->rx_ring, r_vec, idx);
-	} else {
-		r_vec->rx_ring = NULL;
-	}
-
 	snprintf(r_vec->name, sizeof(r_vec->name),
 		 "%s-rxtx-%d", nn->netdev->name, idx);
 	err = request_irq(entry->vector, r_vec->handler, 0, r_vec->name, r_vec);
@@ -2100,6 +2084,9 @@ static int nfp_net_netdev_open(struct net_device *netdev)
 		goto err_free_rx_rings;
 	}
 
+	for (r = 0; r < nn->max_r_vecs; r++)
+		nfp_net_vector_assign_rings(nn, &nn->r_vecs[r], r);
+
 	err = netif_set_real_num_tx_queues(netdev, nn->num_tx_rings);
 	if (err)
 		goto err_free_rings;
@@ -2247,11 +2234,16 @@ static void nfp_net_set_rx_mode(struct net_device *netdev)
 			 struct nfp_net_ring_set *rx,
 			 struct nfp_net_ring_set *tx)
 {
+	unsigned int r;
+
 	if (rx)
 		nfp_net_rx_ring_set_swap(nn, rx);
 	if (tx)
 		nfp_net_tx_ring_set_swap(nn, tx);
 
+	for (r = 0; r <	nn->max_r_vecs; r++)
+		nfp_net_vector_assign_rings(nn, &nn->r_vecs[r], r);
+
 	return __nfp_net_set_config_and_enable(nn);
 }
 
-- 
1.9.1

^ permalink raw reply related
* [PATCH net-next resend 01/13] nfp: add support for ethtool .get_channels
From: Jakub Kicinski @ 2016-11-03 17:11 UTC (permalink / raw)
  To: netdev; +Cc: Jakub Kicinski
In-Reply-To: <1478193129-23476-1-git-send-email-jakub.kicinski@netronome.com>

Report number of rings via ethtool .get_channels API.

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

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 3418f2277e9d..a7386d1b2883 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -614,6 +614,21 @@ static int nfp_net_set_coalesce(struct net_device *netdev,
 	return nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_IRQMOD);
 }
 
+static void nfp_net_get_channels(struct net_device *netdev,
+				 struct ethtool_channels *channel)
+{
+	struct nfp_net *nn = netdev_priv(netdev);
+
+	channel->max_rx = min(nn->max_rx_rings, nn->max_r_vecs);
+	channel->max_tx = min(nn->max_tx_rings, nn->max_r_vecs);
+	channel->max_combined = min(channel->max_rx, channel->max_tx);
+	channel->max_other = NFP_NET_NON_Q_VECTORS;
+	channel->combined_count = min(nn->num_rx_rings, nn->num_tx_rings);
+	channel->rx_count = nn->num_rx_rings - channel->combined_count;
+	channel->tx_count = nn->num_tx_rings - channel->combined_count;
+	channel->other_count = NFP_NET_NON_Q_VECTORS;
+}
+
 static const struct ethtool_ops nfp_net_ethtool_ops = {
 	.get_drvinfo		= nfp_net_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
@@ -632,6 +647,7 @@ static int nfp_net_set_coalesce(struct net_device *netdev,
 	.get_regs		= nfp_net_get_regs,
 	.get_coalesce           = nfp_net_get_coalesce,
 	.set_coalesce           = nfp_net_set_coalesce,
+	.get_channels		= nfp_net_get_channels,
 };
 
 void nfp_net_set_ethtool_ops(struct net_device *netdev)
-- 
1.9.1

^ permalink raw reply related
* [PATCH net-next resend 02/13] nfp: centralize runtime reconfiguration logic
From: Jakub Kicinski @ 2016-11-03 17:11 UTC (permalink / raw)
  To: netdev; +Cc: Jakub Kicinski
In-Reply-To: <1478193129-23476-1-git-send-email-jakub.kicinski@netronome.com>

All functions which need to reallocate ring resources at runtime
look very similar.  Centralize that logic into a separate function.
Encapsulate configuration parameters in a structure.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net.h       |  10 +-
 .../net/ethernet/netronome/nfp/nfp_net_common.c    | 208 +++++++++------------
 .../net/ethernet/netronome/nfp/nfp_net_ethtool.c   |  19 ++
 3 files changed, 118 insertions(+), 119 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index e8713254786b..14b5e21cabf1 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -583,6 +583,12 @@ struct nfp_net {
 	struct dentry *debugfs_dir;
 };
 
+struct nfp_net_ring_set {
+	unsigned int mtu;
+	unsigned int dcnt;
+	void *rings;
+};
+
 /* Functions to read/write from/to a BAR
  * Performs any endian conversion necessary.
  */
@@ -771,7 +777,9 @@ struct nfp_net *
 void nfp_net_coalesce_write_cfg(struct nfp_net *nn);
 int nfp_net_irqs_alloc(struct nfp_net *nn);
 void nfp_net_irqs_disable(struct nfp_net *nn);
-int nfp_net_set_ring_size(struct nfp_net *nn, u32 rxd_cnt, u32 txd_cnt);
+int
+nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_ring_set *rx,
+		      struct nfp_net_ring_set *tx);
 
 #ifdef CONFIG_NFP_NET_DEBUG
 void nfp_net_debugfs_create(void);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 97e0bbef13d1..e58532d27c5b 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -1573,7 +1573,7 @@ static int nfp_net_tx_ring_alloc(struct nfp_net_tx_ring *tx_ring, u32 cnt)
 }
 
 static struct nfp_net_tx_ring *
-nfp_net_shadow_tx_rings_prepare(struct nfp_net *nn, u32 buf_cnt)
+nfp_net_shadow_tx_rings_prepare(struct nfp_net *nn, struct nfp_net_ring_set *s)
 {
 	struct nfp_net_tx_ring *rings;
 	unsigned int r;
@@ -1585,11 +1585,11 @@ static int nfp_net_tx_ring_alloc(struct nfp_net_tx_ring *tx_ring, u32 cnt)
 	for (r = 0; r < nn->num_tx_rings; r++) {
 		nfp_net_tx_ring_init(&rings[r], nn->tx_rings[r].r_vec, r);
 
-		if (nfp_net_tx_ring_alloc(&rings[r], buf_cnt))
+		if (nfp_net_tx_ring_alloc(&rings[r], s->dcnt))
 			goto err_free_prev;
 	}
 
-	return rings;
+	return s->rings = rings;
 
 err_free_prev:
 	while (r--)
@@ -1598,27 +1598,29 @@ static int nfp_net_tx_ring_alloc(struct nfp_net_tx_ring *tx_ring, u32 cnt)
 	return NULL;
 }
 
-static struct nfp_net_tx_ring *
-nfp_net_shadow_tx_rings_swap(struct nfp_net *nn, struct nfp_net_tx_ring *rings)
+static void
+nfp_net_shadow_tx_rings_swap(struct nfp_net *nn, struct nfp_net_ring_set *s)
 {
-	struct nfp_net_tx_ring *old = nn->tx_rings;
+	struct nfp_net_tx_ring *rings = s->rings;
+	struct nfp_net_ring_set new = *s;
 	unsigned int r;
 
+	s->dcnt = nn->txd_cnt;
+	s->rings = nn->tx_rings;
+
 	for (r = 0; r < nn->num_tx_rings; r++)
-		old[r].r_vec->tx_ring = &rings[r];
+		nn->tx_rings[r].r_vec->tx_ring = &rings[r];
 
-	nn->tx_rings = rings;
-	return old;
+	nn->txd_cnt = new.dcnt;
+	nn->tx_rings = new.rings;
 }
 
 static void
-nfp_net_shadow_tx_rings_free(struct nfp_net *nn, struct nfp_net_tx_ring *rings)
+nfp_net_shadow_tx_rings_free(struct nfp_net *nn, struct nfp_net_ring_set *s)
 {
+	struct nfp_net_tx_ring *rings = s->rings;
 	unsigned int r;
 
-	if (!rings)
-		return;
-
 	for (r = 0; r < nn->num_tx_rings; r++)
 		nfp_net_tx_ring_free(&rings[r]);
 
@@ -1691,9 +1693,9 @@ static void nfp_net_rx_ring_free(struct nfp_net_rx_ring *rx_ring)
 }
 
 static struct nfp_net_rx_ring *
-nfp_net_shadow_rx_rings_prepare(struct nfp_net *nn, unsigned int fl_bufsz,
-				u32 buf_cnt)
+nfp_net_shadow_rx_rings_prepare(struct nfp_net *nn, struct nfp_net_ring_set *s)
 {
+	unsigned int fl_bufsz =	nfp_net_calc_fl_bufsz(nn, s->mtu);
 	struct nfp_net_rx_ring *rings;
 	unsigned int r;
 
@@ -1704,14 +1706,14 @@ static void nfp_net_rx_ring_free(struct nfp_net_rx_ring *rx_ring)
 	for (r = 0; r < nn->num_rx_rings; r++) {
 		nfp_net_rx_ring_init(&rings[r], nn->rx_rings[r].r_vec, r);
 
-		if (nfp_net_rx_ring_alloc(&rings[r], fl_bufsz, buf_cnt))
+		if (nfp_net_rx_ring_alloc(&rings[r], fl_bufsz, s->dcnt))
 			goto err_free_prev;
 
 		if (nfp_net_rx_ring_bufs_alloc(nn, &rings[r]))
 			goto err_free_ring;
 	}
 
-	return rings;
+	return s->rings = rings;
 
 err_free_prev:
 	while (r--) {
@@ -1723,27 +1725,32 @@ static void nfp_net_rx_ring_free(struct nfp_net_rx_ring *rx_ring)
 	return NULL;
 }
 
-static struct nfp_net_rx_ring *
-nfp_net_shadow_rx_rings_swap(struct nfp_net *nn, struct nfp_net_rx_ring *rings)
+static void
+nfp_net_shadow_rx_rings_swap(struct nfp_net *nn, struct nfp_net_ring_set *s)
 {
-	struct nfp_net_rx_ring *old = nn->rx_rings;
+	struct nfp_net_rx_ring *rings = s->rings;
+	struct nfp_net_ring_set new = *s;
 	unsigned int r;
 
+	s->mtu = nn->netdev->mtu;
+	s->dcnt = nn->rxd_cnt;
+	s->rings = nn->rx_rings;
+
 	for (r = 0; r < nn->num_rx_rings; r++)
-		old[r].r_vec->rx_ring = &rings[r];
+		nn->rx_rings[r].r_vec->rx_ring = &rings[r];
 
-	nn->rx_rings = rings;
-	return old;
+	nn->netdev->mtu = new.mtu;
+	nn->fl_bufsz = nfp_net_calc_fl_bufsz(nn, new.mtu);
+	nn->rxd_cnt = new.dcnt;
+	nn->rx_rings = new.rings;
 }
 
 static void
-nfp_net_shadow_rx_rings_free(struct nfp_net *nn, struct nfp_net_rx_ring *rings)
+nfp_net_shadow_rx_rings_free(struct nfp_net *nn, struct nfp_net_ring_set *s)
 {
+	struct nfp_net_rx_ring *rings = s->rings;
 	unsigned int r;
 
-	if (!rings)
-		return;
-
 	for (r = 0; r < nn->num_rx_rings; r++) {
 		nfp_net_rx_ring_bufs_free(nn, &rings[r]);
 		nfp_net_rx_ring_free(&rings[r]);
@@ -2255,89 +2262,50 @@ static void nfp_net_set_rx_mode(struct net_device *netdev)
 	nn->ctrl = new_ctrl;
 }
 
-static int nfp_net_change_mtu(struct net_device *netdev, int new_mtu)
+static int
+nfp_net_ring_swap_enable(struct nfp_net *nn,
+			 struct nfp_net_ring_set *rx,
+			 struct nfp_net_ring_set *tx)
 {
-	unsigned int old_mtu, old_fl_bufsz, new_fl_bufsz;
-	struct nfp_net *nn = netdev_priv(netdev);
-	struct nfp_net_rx_ring *tmp_rings;
-	int err;
-
-	old_mtu = netdev->mtu;
-	old_fl_bufsz = nn->fl_bufsz;
-	new_fl_bufsz = nfp_net_calc_fl_bufsz(nn, new_mtu);
-
-	if (!netif_running(netdev)) {
-		netdev->mtu = new_mtu;
-		nn->fl_bufsz = new_fl_bufsz;
-		return 0;
-	}
-
-	/* Prepare new rings */
-	tmp_rings = nfp_net_shadow_rx_rings_prepare(nn, new_fl_bufsz,
-						    nn->rxd_cnt);
-	if (!tmp_rings)
-		return -ENOMEM;
-
-	/* Stop device, swap in new rings, try to start the firmware */
-	nfp_net_close_stack(nn);
-	nfp_net_clear_config_and_disable(nn);
-
-	tmp_rings = nfp_net_shadow_rx_rings_swap(nn, tmp_rings);
-
-	netdev->mtu = new_mtu;
-	nn->fl_bufsz = new_fl_bufsz;
-
-	err = nfp_net_set_config_and_enable(nn);
-	if (err) {
-		const int err_new = err;
+	if (rx)
+		nfp_net_shadow_rx_rings_swap(nn, rx);
+	if (tx)
+		nfp_net_shadow_tx_rings_swap(nn, tx);
 
-		/* Try with old configuration and old rings */
-		tmp_rings = nfp_net_shadow_rx_rings_swap(nn, tmp_rings);
-
-		netdev->mtu = old_mtu;
-		nn->fl_bufsz = old_fl_bufsz;
-
-		err = __nfp_net_set_config_and_enable(nn);
-		if (err)
-			nn_err(nn, "Can't restore MTU - FW communication failed (%d,%d)\n",
-			       err_new, err);
-	}
-
-	nfp_net_shadow_rx_rings_free(nn, tmp_rings);
-
-	nfp_net_open_stack(nn);
+	return __nfp_net_set_config_and_enable(nn);
+}
 
-	return err;
+static void
+nfp_net_ring_reconfig_down(struct nfp_net *nn,
+			   struct nfp_net_ring_set *rx,
+			   struct nfp_net_ring_set *tx)
+{
+	nn->netdev->mtu = rx ? rx->mtu : nn->netdev->mtu;
+	nn->fl_bufsz = nfp_net_calc_fl_bufsz(nn, nn->netdev->mtu);
+	nn->rxd_cnt = rx ? rx->dcnt : nn->rxd_cnt;
+	nn->txd_cnt = tx ? tx->dcnt : nn->txd_cnt;
 }
 
-int nfp_net_set_ring_size(struct nfp_net *nn, u32 rxd_cnt, u32 txd_cnt)
+int
+nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_ring_set *rx,
+		      struct nfp_net_ring_set *tx)
 {
-	struct nfp_net_tx_ring *tx_rings = NULL;
-	struct nfp_net_rx_ring *rx_rings = NULL;
-	u32 old_rxd_cnt, old_txd_cnt;
 	int err;
 
 	if (!netif_running(nn->netdev)) {
-		nn->rxd_cnt = rxd_cnt;
-		nn->txd_cnt = txd_cnt;
+		nfp_net_ring_reconfig_down(nn, rx, tx);
 		return 0;
 	}
 
-	old_rxd_cnt = nn->rxd_cnt;
-	old_txd_cnt = nn->txd_cnt;
-
 	/* Prepare new rings */
-	if (nn->rxd_cnt != rxd_cnt) {
-		rx_rings = nfp_net_shadow_rx_rings_prepare(nn, nn->fl_bufsz,
-							   rxd_cnt);
-		if (!rx_rings)
+	if (rx) {
+		if (!nfp_net_shadow_rx_rings_prepare(nn, rx))
 			return -ENOMEM;
 	}
-	if (nn->txd_cnt != txd_cnt) {
-		tx_rings = nfp_net_shadow_tx_rings_prepare(nn, txd_cnt);
-		if (!tx_rings) {
-			nfp_net_shadow_rx_rings_free(nn, rx_rings);
-			return -ENOMEM;
+	if (tx) {
+		if (!nfp_net_shadow_tx_rings_prepare(nn, tx)) {
+			err = -ENOMEM;
+			goto err_free_rx;
 		}
 	}
 
@@ -2345,39 +2313,43 @@ int nfp_net_set_ring_size(struct nfp_net *nn, u32 rxd_cnt, u32 txd_cnt)
 	nfp_net_close_stack(nn);
 	nfp_net_clear_config_and_disable(nn);
 
-	if (rx_rings)
-		rx_rings = nfp_net_shadow_rx_rings_swap(nn, rx_rings);
-	if (tx_rings)
-		tx_rings = nfp_net_shadow_tx_rings_swap(nn, tx_rings);
-
-	nn->rxd_cnt = rxd_cnt;
-	nn->txd_cnt = txd_cnt;
-
-	err = nfp_net_set_config_and_enable(nn);
+	err = nfp_net_ring_swap_enable(nn, rx, tx);
 	if (err) {
-		const int err_new = err;
-
-		/* Try with old configuration and old rings */
-		if (rx_rings)
-			rx_rings = nfp_net_shadow_rx_rings_swap(nn, rx_rings);
-		if (tx_rings)
-			tx_rings = nfp_net_shadow_tx_rings_swap(nn, tx_rings);
+		int err2;
 
-		nn->rxd_cnt = old_rxd_cnt;
-		nn->txd_cnt = old_txd_cnt;
+		nfp_net_clear_config_and_disable(nn);
 
-		err = __nfp_net_set_config_and_enable(nn);
-		if (err)
+		/* Try with old configuration and old rings */
+		err2 = nfp_net_ring_swap_enable(nn, rx, tx);
+		if (err2)
 			nn_err(nn, "Can't restore ring config - FW communication failed (%d,%d)\n",
-			       err_new, err);
+			       err, err2);
 	}
 
-	nfp_net_shadow_rx_rings_free(nn, rx_rings);
-	nfp_net_shadow_tx_rings_free(nn, tx_rings);
+	if (rx)
+		nfp_net_shadow_rx_rings_free(nn, rx);
+	if (tx)
+		nfp_net_shadow_tx_rings_free(nn, tx);
 
 	nfp_net_open_stack(nn);
 
 	return err;
+
+err_free_rx:
+	if (rx)
+		nfp_net_shadow_rx_rings_free(nn, rx);
+	return err;
+}
+
+static int nfp_net_change_mtu(struct net_device *netdev, int new_mtu)
+{
+	struct nfp_net *nn = netdev_priv(netdev);
+	struct nfp_net_ring_set rx = {
+		.mtu = new_mtu,
+		.dcnt = nn->rxd_cnt,
+	};
+
+	return nfp_net_ring_reconfig(nn, &rx, NULL);
 }
 
 static struct rtnl_link_stats64 *nfp_net_stat64(struct net_device *netdev,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index a7386d1b2883..3f48256dc03c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -158,6 +158,25 @@ static void nfp_net_get_ringparam(struct net_device *netdev,
 	ring->tx_pending = nn->txd_cnt;
 }
 
+static int nfp_net_set_ring_size(struct nfp_net *nn, u32 rxd_cnt, u32 txd_cnt)
+{
+	struct nfp_net_ring_set *reconfig_rx = NULL, *reconfig_tx = NULL;
+	struct nfp_net_ring_set rx = {
+		.mtu = nn->netdev->mtu,
+		.dcnt = rxd_cnt,
+	};
+	struct nfp_net_ring_set tx = {
+		.dcnt = txd_cnt,
+	};
+
+	if (nn->rxd_cnt != rxd_cnt)
+		reconfig_rx = &rx;
+	if (nn->txd_cnt != txd_cnt)
+		reconfig_tx = &tx;
+
+	return nfp_net_ring_reconfig(nn, reconfig_rx, reconfig_tx);
+}
+
 static int nfp_net_set_ringparam(struct net_device *netdev,
 				 struct ethtool_ringparam *ring)
 {
-- 
1.9.1

^ permalink raw reply related
* [PATCH net-next resend 00/13] ring reconfiguration and XDP support
From: Jakub Kicinski @ 2016-11-03 17:11 UTC (permalink / raw)
  To: netdev; +Cc: Jakub Kicinski

Hi!

This set adds support for ethtool channel API and XDP.

I kick off with ethtool get_channels() implementation.  
set_channels() needs some preparations to get right.  I follow
the prepare/commit paradigm and allocate all resources before
stopping the device.  It has already been done for ndo_change_mtu
and ethtool set_ringparam(), it makes sense now to consolidate all
the required logic in one place.

XDP support requires splitting TX rings into two classes - 
for the stack and for XDP.  The ring structures are identical.
The differences are in how they are connected to IRQ vector
structs and how the completion/cleanup works.  When XDP is enabled
I switch from the frag allocator to page-per-packet and map buffers
BIDIRECTIONALly.

Last but not least XDP offload is added (the patch just takes
care of the small formal differences between cls_bpf and XDP).

There is a tiny & trivial DebugFS patch in the mix, I hope it can
be taken via net-next provided we have the right Acks.

Resending with improved commit message and CCing more people on patch 10.

Jakub Kicinski (13):
  nfp: add support for ethtool .get_channels
  nfp: centralize runtime reconfiguration logic
  nfp: rename ring allocation helpers
  nfp: reuse ring helpers on .ndo_open() path
  nfp: loosen relation between rings and IRQs vectors
  nfp: add helper to reassign rings to IRQ vectors
  nfp: move RSS indirection table init into a separate function
  nfp: add support for ethtool .set_channels
  nfp: reorganize nfp_net_rx() to get packet offsets early
  debugfs: constify argument to debugfs_real_fops()
  nfp: add XDP support in the driver
  nfp: remove unnecessary parameters from nfp_net_bpf_offload()
  nfp: add support for offload of XDP programs

 drivers/net/ethernet/netronome/nfp/nfp_bpf.h       |   1 +
 drivers/net/ethernet/netronome/nfp/nfp_bpf_jit.c   |  92 ++-
 .../net/ethernet/netronome/nfp/nfp_bpf_verifier.c  |   3 +
 drivers/net/ethernet/netronome/nfp/nfp_net.h       |  30 +-
 .../net/ethernet/netronome/nfp/nfp_net_common.c    | 892 ++++++++++++++-------
 .../net/ethernet/netronome/nfp/nfp_net_debugfs.c   |  41 +-
 .../net/ethernet/netronome/nfp/nfp_net_ethtool.c   |  94 +++
 .../net/ethernet/netronome/nfp/nfp_net_offload.c   |   7 +-
 include/linux/debugfs.h                            |   3 +-
 9 files changed, 869 insertions(+), 294 deletions(-)

-- 
1.9.1

^ permalink raw reply
* Re: net/sctp: use-after-free in __sctp_connect
From: Andrey Konovalov @ 2016-11-03 17:11 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Eric Dumazet, Dmitry Vyukov
In-Reply-To: <CAAeHK+xaBVcZp7PfH-tYWaxs6rCiaNAivZj4xZLcawFShi6C1Q@mail.gmail.com>

On Wed, Nov 2, 2016 at 11:42 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Wed, Oct 19, 2016 at 6:57 PM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
>> On Wed, Oct 19, 2016 at 02:25:24PM +0200, Andrey Konovalov wrote:
>>> Hi,
>>>
>>> I've got the following error report while running the syzkaller fuzzer:
>>>
>>> ==================================================================
>>> BUG: KASAN: use-after-free in __sctp_connect+0xabe/0xbf0 at addr
>>> ffff88006b1dc610
>>
>> Seems this is the same that Dmitry Vyukov had reported back in Jan 13th.
>> So far I couldn't identify the reason.
>> "Good" to know it's still there, thanks for reporting it.

Hi Marcelo,

So I've looked at the code.
As far as I understand, the problem is a race condition between
setsockopt(SCTP_SOCKOPT_CONNECTX) and shutdown on an sctp socket.
setsockopt() calls sctp_wait_for_connect(), which exits the for loop
on the sk->sk_shutdown & RCV_SHUTDOWN if clause, and then frees asoc
with sctp_association_put() and returns err = 0.
Then __sctp_connect() checks that err == 0 and reads asoc->assoc_id
from the freed asoc.

^ permalink raw reply
* Re: [PATCH RFC 0/2] ethtool: Add actual port speed reporting
From: Rick Jones @ 2016-11-03 17:09 UTC (permalink / raw)
  To: Gal Pressman, Mintz, Yuval, Gal Pressman, netdev@vger.kernel.org,
	John W. Linville, Vidya Sagar Ravipati, Saeed Mahameed
  Cc: David Decotigny, Ben Hutchings
In-Reply-To: <7a681afa-a5fa-3039-873c-d13617484cc9@gmail.com>

>> And besides, one can argue that in the SR-IOV scenario the VF has no business
>> knowing the physical port speed.
>>
>
> Good point, but there are more use-cases we should consider.
> For example, when using Multi-Host/Flex-10/Multi-PF each PF should
> be able to query both physical port speed and actual speed.

Despite my email address, I'm not fully versed on VC/Flex, but I have 
always been under the impression that the flexnics created were, 
conceptually, "distinct" NICs considered independently of the physical 
port over which they operated.  Tossing another worm or three into the 
can, while "back in the day" (when some of the first ethtool changes to 
report speeds other than the "normal" ones went in) the speed of a 
flexnic was fixed, today, it can actually operate in a range.  From a 
minimum guarantee to an "if there is bandwidth available" cap.

rick jones

^ permalink raw reply
* Re: [PATCH RFC 0/2] ethtool: Add actual port speed reporting
From: Gal Pressman @ 2016-11-03 16:59 UTC (permalink / raw)
  To: Mintz, Yuval, Gal Pressman, netdev@vger.kernel.org,
	John W. Linville, Vidya Sagar Ravipati, Saeed Mahameed
  Cc: David Decotigny, Ben Hutchings
In-Reply-To: <BL2PR07MB23069B0DB17B981AA43775B18DA00@BL2PR07MB2306.namprd07.prod.outlook.com>



On 02/11/2016 17:50, Mintz, Yuval wrote:
>> Sending RFC to get feedback for the following ethtool proposal:
>>
>> In some cases such as virtual machines and multi functions (SR-IOV), the actual
>> bandwidth exposed for each machine is not accurately shown in ethtool.
>> Currently ethtool shows only physical port link speed.
>> In our case we would like to show the virtual port operational link speed which
>> in some cases is less than the physical port speed.
>>
>> This will give users better visibility for the actual speed running on their device.
>>
>> $ ethtool ens6
>> ...
>> Speed: 50000Mb/s
>> Actual speed: 25000Mb/s
> 
> Not saying this is a bad thing, but where exactly is it listed that ethtool has
> to show the physical port speed?
> E.g., bnx2x shows the logical speed instead, and has been doing that for years.
> [Perhaps that's a past wrongness, but that's how it goes].
> 
> And besides, one can argue that in the SR-IOV scenario the VF has no business
> knowing the physical port speed.
> 

Good point, but there are more use-cases we should consider.
For example, when using Multi-Host/Flex-10/Multi-PF each PF should
be able to query both physical port speed and actual speed.

^ permalink raw reply
* Re: [PATCH net-next 10/13] debugfs: constify argument to debugfs_real_fops()
From: Jakub Kicinski @ 2016-11-03 16:58 UTC (permalink / raw)
  To: Nicolai Stange; +Cc: netdev, Christian Lamparter
In-Reply-To: <87zilgv6yc.fsf@gmail.com>

On Thu, 03 Nov 2016 17:55:39 +0100, Nicolai Stange wrote:
> Hi Jakub,
> 
> thanks for this.
> 
> However, the debugfs maintainer, Greg K-H, as well as the lkml is
> missing from the To/Cc. Can you resend please?

Sure thing!

> Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> 
> > seq_file users can only access const version of file pointer,  
> 
> ... because the ->file member of struct seq_operations is marked as such.

Thanks!

^ permalink raw reply
* Re: stmmac/RTL8211F/Meson GXBB: TX throughput problems
From: Jerome Brunet @ 2016-11-03 16:57 UTC (permalink / raw)
  To: André Roth, Martin Blumenstingl
  Cc: Johnson Leung, Giuseppe CAVALLARO, linux-amlogic,
	Alexandre Torgue, netdev
In-Reply-To: <20161031112524.4c4ee8f8@gmail.com>

On Mon, 2016-10-31 at 11:25 +0100, André Roth wrote:
> Hi all,
>  
> > 
> > on my device this results in:
> > [0xc9410018] = 0x2000000
> > [0xc9410030] = 0x0
> > [0xc941003c] = 0x0
> > [0xc9411000] = 0x1100802
> > [0xc9411018] = 0x2202006
> > [0xc9411028] = 0x0
> > 
> > maybe someone else could try the command from above on his device
> > (running the original Amlogic kernel).
> 
> those registers have the same value on an original image from
> hardkernel: 
> 
> Linux odroid64 3.14.65-65 #1 SMP PREEMPT Sat May 28
> 02:50:51 BRT 2016 aarch64 aarch64 aarch64 GNU/Linux
> 
> > 
> > please also state if ethernet is working properly on the original
> > kernel (and preferably which device/board this is).
> 
> yes, the ethernet works flawless in 100 and 1000 Mbit/s on the 3.14
> kernel.

Andre, the 3.14 kernel you are talking, is it this one ? : 
https://github.com/hardkernel/linux/tree/odroidc2-3.14.y

Because in drivers/net/phy/realtek.c, they disable EEE, but
also 1000Base-T Full Duplex advertisement ?

+	/* disable 1000m adv*/
+	val = phy_read(phydev, 0x9);
+	phy_write(phydev, 0x9, val&(~(1<<9)));

If this is the kernel you are running, you should not be able to have
ethernet at 1000MB/s ? Or is it in half duplex mode ?

> 
> I can now confirm that both 100 and 1000 Mbit/s do not work properly
> on the 4.8/integ branch. the network connection is interrupted after
> some outbound traffic. it can be recovered by running a ifdown/ifup
> which restarts dhclient, which I think is able to somehow reset the
> interface. 
> 
> Anything I can help to debug the issue further ?
> 
> Regards,
> 
>  André
>  
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply
* Re: [PATCH net-next 10/13] debugfs: constify argument to debugfs_real_fops()
From: Nicolai Stange @ 2016-11-03 16:55 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, Nicolai Stange, Christian Lamparter
In-Reply-To: <1478181538-20778-11-git-send-email-jakub.kicinski@netronome.com>

Hi Jakub,

thanks for this.

However, the debugfs maintainer, Greg K-H, as well as the lkml is
missing from the To/Cc. Can you resend please?

Jakub Kicinski <jakub.kicinski@netronome.com> writes:

> seq_file users can only access const version of file pointer,

... because the ->file member of struct seq_operations is marked as such.

> make parameter to debugfs_real_fops() const.
>
> CC: Nicolai Stange <nicstange@gmail.com>
> CC: Christian Lamparter <chunkeey@gmail.com>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>  include/linux/debugfs.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> index 4d3f0d1aec73..bf1907d96097 100644
> --- a/include/linux/debugfs.h
> +++ b/include/linux/debugfs.h
> @@ -52,7 +52,8 @@ struct debugfs_regset32 {
>   * Must only be called under the protection established by
>   * debugfs_use_file_start().
>   */
> -static inline const struct file_operations *debugfs_real_fops(struct file *filp)
> +static inline const struct file_operations *
> +debugfs_real_fops(const struct file *filp)
>  	__must_hold(&debugfs_srcu)
>  {
>  	/*

^ permalink raw reply
* Re: [PATCH v5 5/7] net: ethernet: bgmac: device tree phy enablement
From: Jon Mason @ 2016-11-03 16:45 UTC (permalink / raw)
  To: rafal-g1n6cQUeyibVItvQsEIGlw
  Cc: David Miller, Rob Herring, Mark Rutland, Florian Fainelli,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <35b16640-8600-8c48-2f04-886dc925229d-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

On Thu, Nov 03, 2016 at 09:31:21AM +0100, Rafal Milecki wrote:
> On 11/02/2016 06:08 PM, Jon Mason wrote:
> >Change the bgmac driver to allow for phy's defined by the device tree
> 
> This is a late review, I know, sorry... :(
> 
> 
> >+static int bcma_phy_direct_connect(struct bgmac *bgmac)
> >+{
> >+	struct fixed_phy_status fphy_status = {
> >+		.link = 1,
> >+		.speed = SPEED_1000,
> >+		.duplex = DUPLEX_FULL,
> >+	};
> >+	struct phy_device *phy_dev;
> >+	int err;
> >+
> >+	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
> >+	if (!phy_dev || IS_ERR(phy_dev)) {
> >+		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
> >+		return -ENODEV;
> >+	}
> >+
> >+	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
> >+				 PHY_INTERFACE_MODE_MII);
> >+	if (err) {
> >+		dev_err(bgmac->dev, "Connecting PHY failed\n");
> >+		return err;
> >+	}
> >+
> >+	return err;
> >+}
> 
> This bcma specific function looks exactly the same as...
> 
> 
> >+static int platform_phy_direct_connect(struct bgmac *bgmac)
> >+{
> >+	struct fixed_phy_status fphy_status = {
> >+		.link = 1,
> >+		.speed = SPEED_1000,
> >+		.duplex = DUPLEX_FULL,
> >+	};
> >+	struct phy_device *phy_dev;
> >+	int err;
> >+
> >+	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
> >+	if (!phy_dev || IS_ERR(phy_dev)) {
> >+		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
> >+		return -ENODEV;
> >+	}
> >+
> >+	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
> >+				 PHY_INTERFACE_MODE_MII);
> >+	if (err) {
> >+		dev_err(bgmac->dev, "Connecting PHY failed\n");
> >+		return err;
> >+	}
> >+
> >+	return err;
> >+}
> 
> This one.
> 
> Would that make sense to keep bgmac_phy_connect_direct and just use it in
> bcma/platform code?

Yes, I was having the same internal debate.  I hate the duplication of
code, but I really wanted to keep the PHY logic out of the bgmac.c
file.  Do you think it is acceptable to make this an inline function
in bgmac.h?

Thanks,
Jon
--
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
* [Patch net] taskstats: fix the length of cgroupstats_cmd_get_policy
From: Cong Wang @ 2016-11-03 16:42 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang
In-Reply-To: <1478191356-10386-1-git-send-email-xiyou.wangcong@gmail.com>

cgroupstats_cmd_get_policy is [CGROUPSTATS_CMD_ATTR_MAX+1],
taskstats_cmd_get_policy[TASKSTATS_CMD_ATTR_MAX+1],
but their family.maxattr is TASKSTATS_CMD_ATTR_MAX.
CGROUPSTATS_CMD_ATTR_MAX is less than TASKSTATS_CMD_ATTR_MAX,
so we could end up accessing out-of-bound.

Change cgroupstats_cmd_get_policy to TASKSTATS_CMD_ATTR_MAX+1,
this is safe because the rest are initialized to 0's.

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 kernel/taskstats.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index b3f05ee..cbb387a 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -54,7 +54,11 @@ static const struct nla_policy taskstats_cmd_get_policy[TASKSTATS_CMD_ATTR_MAX+1
 	[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK] = { .type = NLA_STRING },
 	[TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK] = { .type = NLA_STRING },};
 
-static const struct nla_policy cgroupstats_cmd_get_policy[CGROUPSTATS_CMD_ATTR_MAX+1] = {
+/*
+ * We have to use TASKSTATS_CMD_ATTR_MAX here, it is the maxattr in the family.
+ * Make sure they are always aligned.
+ */
+static const struct nla_policy cgroupstats_cmd_get_policy[TASKSTATS_CMD_ATTR_MAX+1] = {
 	[CGROUPSTATS_CMD_ATTR_FD] = { .type = NLA_U32 },
 };
 
-- 
2.1.0

^ permalink raw reply related
* [Patch net] genetlink: fix a memory leak on error path
From: Cong Wang @ 2016-11-03 16:42 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Jakub Kicinski, Johannes Berg

In __genl_register_family(), when genl_validate_assign_mc_groups()
fails, we forget to free the memory we possibly allocate for
family->attrbuf.

Note, some callers call genl_unregister_family() to clean up
on error path, it doesn't work because the family is inserted
to the global list in the nearly last step.

Cc: Jakub Kicinski <kubakici@wp.pl>
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/netlink/genetlink.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 23cc126..49c28e8 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -404,7 +404,7 @@ int __genl_register_family(struct genl_family *family)
 
 	err = genl_validate_assign_mc_groups(family);
 	if (err)
-		goto errout_locked;
+		goto errout_free;
 
 	list_add_tail(&family->family_list, genl_family_chain(family->id));
 	genl_unlock_all();
@@ -417,6 +417,8 @@ int __genl_register_family(struct genl_family *family)
 
 	return 0;
 
+errout_free:
+	kfree(family->attrbuf);
 errout_locked:
 	genl_unlock_all();
 errout:
-- 
2.1.0

^ permalink raw reply related
* Is there a maximum bytes in flight limitation in the tcp stack?
From: De Schepper, Koen (Nokia - BE) @ 2016-11-03 16:37 UTC (permalink / raw)
  To: netdev@vger.kernel.org

Hi,

We experience some limit on the maximum packets in flight which seem not to be related with the receive or write buffers. Does somebody know if there is an issue with a maximum of around 1MByte (or sometimes 2Mbyte) of data in flight per TCP flow?

It seems to be a strict and stable limit independent from the CC (tested with Cubic, Reno and DCTCP). On a link of 200Mbps and 200ms RTT our link is only 20% (sometimes 40%, see conditions below) utilized for a single TCP flow with no drop experienced at all (no bottleneck in the AQM or RTT emulation, as it supports more throughput if multiple flows are active).

Some configuration changes we already tried on both client and server (kernel 3.18.9):

net.ipv4.tcp_no_metrics_save = 1
net.ipv4.tcp_rmem = 4096 87380 6291456
net.ipv4.tcp_wmem = 4096 16384 4194304

SERVER# ss -i
tcp    ESTAB      0      1049728  10.187.255.211:46642     10.187.16.194:ssh
	 dctcp wscale:7,7 rto:408 rtt:204.333/0.741 ato:40 mss:1448 cwnd:1466 send 83.1Mbps unacked:728 rcv_rtt:212 rcv_space:29200
CLIENT# ss -i
tcp    ESTAB      0      288      10.187.16.194:ssh      10.187.255.211:46642
	 dctcp wscale:7,7 rto:404 rtt:203.389/0.213 ato:40 mss:1448 cwnd:78 send 4.4Mbps unacked:8 rcv_rtt:204 rcv_space:1074844

When increasing the write and receive mem further (they were already way above 1 or 2 MB) it steps to double (40%; 2Mbytes in flight):
net.ipv4.tcp_no_metrics_save = 1
net.ipv4.tcp_rmem = 4096 8000000 16291456
net.ipv4.tcp_wmem = 4096 8000000 16291456

SERVER # ss -i
tcp    ESTAB      0      2068976  10.187.255.212:54637     10.187.16.112:ssh
	 cubic wscale:8,8 rto:404 rtt:202.622/0.061 ato:40 mss:1448 cwnd:1849 ssthresh:1140 send 105.7Mbps unacked:1457 rcv_rtt:217.5 rcv_space:29200
CLIENT# ss -i
tcp    ESTAB      0      648      10.187.16.112:ssh      10.187.255.212:54637
	 cubic wscale:8,8 rto:404 rtt:201.956/0.038 ato:40 mss:1448 cwnd:132 send 7.6Mbps unacked:18 rcv_rtt:204 rcv_space:2093044

Further increasing (x10) does not help anymore...
net.ipv4.tcp_no_metrics_save = 1
net.ipv4.tcp_rmem = 4096 80000000 162914560
net.ipv4.tcp_wmem = 4096 80000000 162914560

As all these parameters autotune, it is hard to find out which one is limiting... In the examples, above unacked does not want to go higher, while congestion window in the server is big enough... rcv_space could be limiting, but it tunes up if I change the server with the higher buffers (switching to 2MByte in flight).

We also tried tcp_limit_output_bytes, setting it bigger (x10) and smaller(/10), without effect. We've put it in /etc/sysctl.conf and rebooted, to make sure that it is effective.

Some more detailed tests that had an effect on the 1 or 2MByte:
- It seems that with TSO off, if we configure a bigger wmem buffer, an ongoing flow suddenly is able to immediately double its bytes in flight limit. We configured further up to more than 10x the buffer, but no further increase helps, and the limits we saw are only 1MByte and 2Mbyte (no intermediate values depending on any parameter). When setting tcp_wmem smaller again, the 2MByte limit stays on the ongoing flow. We have to restart the flow to make the buffer reduction to 1MByte effective.
- With TSO on, only the 2MByte limit is effective, independent from the wmem buffer. We have to restart the flow to make a tso change effective.

Koen.

^ permalink raw reply
* Re: stmmac/RTL8211F/Meson GXBB: TX throughput problems
From: Jerome Brunet @ 2016-11-03 16:36 UTC (permalink / raw)
  To: Martin Blumenstingl, Giuseppe CAVALLARO
  Cc: Johnson Leung, netdev, André Roth, Alexandre Torgue,
	linux-amlogic
In-Reply-To: <CAFBinCD_-KjVa70iOhBSGaXZc9uKy2m4d2yVr-AfLiV7F=3EqQ@mail.gmail.com>

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

On Sat, 2016-10-01 at 17:58 +0200, Martin Blumenstingl wrote:
> Hello Peppe,
> 
> On Mon, Sep 26, 2016 at 8:17 AM, Giuseppe CAVALLARO
> <peppe.cavallaro@st.com> wrote:
> > 
> > Hello André
> > 
> > On 9/17/2016 11:23 PM, André Roth wrote:
> > > 
> > > 
> > > 
> > > Hi all,
> > > 
> > > I have an odroid c2 board which shows this issue. No data is
> > > transmitted or received after a moment of intense tx traffic.
> > > Copying a
> > > 1GB file per scp from the board triggers it repeatedly.
> > > 
> > > The board has a stmmac - user ID: 0x11, Synopsys ID: 0x37.
> > > 
> > > When switching the network to 100Mb/s the copying does
> > > not seam to trigger the issue.
> > > 
> > > I've attached the ethtool statistics before and after the
> > > problem.
> > 
> > 
> > at first glance, it enters in EEE mode often in the ethtool.after.
> > On some platforms we met problems and it was necessary to disable
> > the
> > feature. Maybe, you can start looking at if this is true on yours.
> > We will see to provide a clean subset of patches to switch-on/off
> > it.
> I did some hacking in the stmmac driver to disable the LPI stuff (see
> the attachment)
> 
> Unfortunately this did not fix the problem.
> 
> I did not issue any ethtool commands not shown in the logs.
> Also I did not have time to change the AXI tuning / PBL value yet -
> so
> those are also untouched.
> 
> I will keep testing, but unfortunately my device is starting to fall
> apart (I sometimes have DDR initialization issues and u-boot fails to
> come up, oh dear...).

Hi all,

I did several tests on this issue with amlogic's S905 SoC (Synopsys MAC
- user ID: 0x11, Synopsys ID: 0x37.) 

With the OdroidC2 (PHY Realtek RTL8211F), EEE is on by default.
Just before launching iperf3, here are the ethtool stats regarding LPI:
     irq_tx_path_in_lpi_mode_n: 6
     irq_tx_path_exit_lpi_mode_n: 5
     irq_rx_path_in_lpi_mode_n: 76
     irq_rx_path_exit_lpi_mode_n: 75
     phy_eee_wakeup_error_n: 0

Sending data with iperf usually works for little while (between 0 and
10s)

# iperf3 -c 192.168.1.170 -p12345
Connecting to host 192.168.1.170, port 12345
local 192.168.1.30 port 54450 connected to 192.168.1.170 port 12345
Interval           Transfer     Bandwidth       Retr  Cwnd
0.00-1.00   sec   112 MBytes   938 Mbits/sec    0    409 KBytes       
1.00-2.00   sec   112 MBytes   940 Mbits/sec    0    426 KBytes       
2.00-3.00   sec   112 MBytes   939 Mbits/sec    0    426 KBytes       
3.00-4.00   sec   112 MBytes   940 Mbits/sec    0    426 KBytes       
4.00-5.00   sec   112 MBytes   940 Mbits/sec    0    426 KBytes       
5.00-6.00   sec   112 MBytes   939 Mbits/sec    0    426 KBytes       
6.00-7.00   sec  9.26 MBytes  77.6 Mbits/sec    2   1.41 KBytes       
7.00-8.00   sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes       
8.00-9.00   sec  0.00 Bytes  0.00 bits/sec    0   1.41 KBytes       
^C10.00-13.58  sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
Interval           Transfer     Bandwidth       Retr
0.00-13.58  sec   681 MBytes   421 Mbits/sec    4             sender
0.00-13.58  sec  0.00 Bytes  0.00 bits/sec                  receiver
iperf3: interrupt - the client has terminated

iperf3 does not exit ant the link seems completely broken. We cannot
send or receive until the interface is brought down then up again.

Here are the LPI related stats after the test:
     irq_tx_path_in_lpi_mode_n: 48
     irq_tx_path_exit_lpi_mode_n: 48
     irq_rx_path_in_lpi_mode_n: 325
     irq_rx_path_exit_lpi_mode_n: 325
     phy_eee_wakeup_error_n: 0

Like Martin, I tried playing around with eee in stmmac, but I could not
improve the situation. Then I tried disabling EEE advertisement on the
PHY (patch attached). With this patch, iperf3 runs nicely for me.

This is what the folks of FreeBSD have done for the Same MAC/PHY
combination [0]

On the P200 Board (PHY Micrel KSZ9031), EEE is off by default. There is
no problem on this board right now. I tried to force the activation of
EEE on this board and ended up in the same situation as the OdroidC2
(link broken). The stats were a bit different though:
     irq_tx_path_in_lpi_mode_n: 28
     irq_tx_path_exit_lpi_mode_n: 28
     irq_rx_path_in_lpi_mode_n: 408
     irq_rx_path_exit_lpi_mode_n: 408
     phy_eee_wakeup_error_n: 5440

To everybody having similar issue with their OdroidC2, could you try
the attached patch and let us know if it changes anything for you ?

Peppe, Alexandre,
What is your view on this ? I'm not sure that removing EEE
advertisement is the right way to address the problem ?
Could it be an issue stmmac ?
If there is any other information / test which would help understand
the issue, please let me know.

Cheers

Jerome


[0] : https://github.com/freebsd/freebsd-base-graphics/commit/1f49e276c
3801545dc0a337792a5f07e6ad39c84
 

> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

[-- Attachment #2: realtek8211f-disable-eee-1000.patch --]
[-- Type: text/x-patch, Size: 2912 bytes --]

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index a45d1013c225..3cbeec63a439 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -127,3 +127,18 @@
 &usb1 {
 	status = "okay";
 };
+
+&ethmac {
+	phy-handle = <&eth_phy0>;
+
+	mdio {
+		compatible = "snps,dwmac-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		eth_phy0: ethernet-phy@0 {
+			reg = <0>;
+			realtek,disable-eee-1000t;
+		};
+	};
+};
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index aadd6e9f54ad..30e20ba10f45 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -15,6 +15,12 @@
  */
 #include <linux/phy.h>
 #include <linux/module.h>
+#include <linux/of.h>
+
+struct rtl8211f_phy_priv {
+	bool eee_1000_disable;
+	bool eee_100_disable;
+};
 
 #define RTL821x_PHYSR		0x11
 #define RTL821x_PHYSR_DUPLEX	0x2000
@@ -93,6 +99,25 @@ static int rtl8211f_config_intr(struct phy_device *phydev)
 	return err;
 }
 
+static void rtl8211f_force_eee(struct phy_device *phydev)
+{
+	struct rtl8211f_phy_priv *priv = phydev->priv;
+	u16 val;
+
+	if (priv->eee_1000_disable || priv->eee_100_disable) {
+		val = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV,
+					    MDIO_MMD_AN);
+
+		if (priv->eee_1000_disable)
+			val &= ~MDIO_AN_EEE_ADV_1000T;
+		if (priv->eee_100_disable)
+			val &= ~MDIO_AN_EEE_ADV_100TX;
+
+		phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV,
+				       MDIO_MMD_AN, val);
+	}
+}
+
 static int rtl8211f_config_init(struct phy_device *phydev)
 {
 	int ret;
@@ -102,6 +127,8 @@ static int rtl8211f_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
+	rtl8211f_force_eee(phydev);
+
 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII) {
 		/* enable TXDLY */
 		phy_write(phydev, RTL8211F_PAGE_SELECT, 0xd08);
@@ -115,6 +142,26 @@ static int rtl8211f_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int rtl8211f_phy_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct device_node *of_node = dev->of_node;
+	struct rtl8211f_phy_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (of_property_read_bool(of_node, "realtek,disable-eee-1000t"))
+		priv->eee_1000_disable= true;
+	if (of_property_read_bool(of_node, "realtek,disable-eee-100t"))
+		priv->eee_100_disable= true;
+
+	phydev->priv = priv;
+
+	return 0;
+}
+
 static struct phy_driver realtek_drvs[] = {
 	{
 		.phy_id         = 0x00008201,
@@ -164,6 +211,7 @@ static struct phy_driver realtek_drvs[] = {
 		.phy_id_mask	= 0x001fffff,
 		.features	= PHY_GBIT_FEATURES,
 		.flags		= PHY_HAS_INTERRUPT,
+		.probe		= &rtl8211f_phy_probe,
 		.config_aneg	= &genphy_config_aneg,
 		.config_init	= &rtl8211f_config_init,
 		.read_status	= &genphy_read_status,

^ permalink raw reply related
* Re: bpf: kernel BUG in htab_elem_free
From: Daniel Borkmann @ 2016-11-03 16:36 UTC (permalink / raw)
  To: Dmitry Vyukov, Alexei Starovoitov, netdev; +Cc: LKML, syzkaller
In-Reply-To: <CACT4Y+a2W++HSgMXqFSqNDuexudHf0buZqtt7pBHrAKz57pFeQ@mail.gmail.com>

On 11/03/2016 03:15 PM, Dmitry Vyukov wrote:
> On Wed, Nov 2, 2016 at 11:14 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> Here we go.
>>
>> The following program triggers kernel BUG in htab_elem_free.
>> On commit 0c183d92b20b5c84ca655b45ef57b3318b83eb9e (Oct 31).
>> Run as "while true; do ./a.out; done".

This one fixes it for me. Could you check it from your side as well?
I'll submit an official fix then.

Thanks a lot for the catch!
Daniel

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 570eeca..ad1bc67 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -687,7 +687,8 @@ static void delete_all_elements(struct bpf_htab *htab)

  		hlist_for_each_entry_safe(l, n, head, hash_node) {
  			hlist_del_rcu(&l->hash_node);
-			htab_elem_free(htab, l);
+			if (l->state != HTAB_EXTRA_ELEM_USED)
+				htab_elem_free(htab, l);
  		}
  	}
  }

^ permalink raw reply related
* [PATCH net-next] net: Update raw socket bind to consider l3 domain
From: David Ahern @ 2016-11-03 16:25 UTC (permalink / raw)
  To: netdev; +Cc: David Ahern

Binding a raw socket to a local address fails if the socket is bound
to an L3 domain:

    $ vrf-test  -s -l 10.100.1.2 -R -I red
    error binding socket: 99: Cannot assign requested address

Update raw_bind to look consider if sk_bound_dev_if is bound to an L3
domain and use inet_addr_type_table to lookup the address.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 net/ipv4/raw.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 6a0bd68a565b..9ef2a602f052 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -695,12 +695,20 @@ static int raw_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 {
 	struct inet_sock *inet = inet_sk(sk);
 	struct sockaddr_in *addr = (struct sockaddr_in *) uaddr;
+	u32 tb_id = RT_TABLE_LOCAL;
 	int ret = -EINVAL;
 	int chk_addr_ret;
 
 	if (sk->sk_state != TCP_CLOSE || addr_len < sizeof(struct sockaddr_in))
 		goto out;
-	chk_addr_ret = inet_addr_type(sock_net(sk), addr->sin_addr.s_addr);
+
+	if (sk->sk_bound_dev_if)
+		tb_id = l3mdev_fib_table_by_index(sock_net(sk),
+						 sk->sk_bound_dev_if) ? : tb_id;
+
+	chk_addr_ret = inet_addr_type_table(sock_net(sk), addr->sin_addr.s_addr,
+					    tb_id);
+
 	ret = -EADDRNOTAVAIL;
 	if (addr->sin_addr.s_addr && chk_addr_ret != RTN_LOCAL &&
 	    chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST)
-- 
2.1.4

^ permalink raw reply related
* Re: [PATCH net] ipv6: dccp: add missing bind_conflict to dccp_ipv6_mapped
From: Arnaldo Carvalho de Melo @ 2016-11-03 16:22 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Andrey Konovalov, netdev, syzkaller, Gerrit Renker,
	dccp, Arnaldo Carvalho de Melo
In-Reply-To: <1478188786.7065.453.camel@edumazet-glaptop3.roam.corp.google.com>

Em Thu, Nov 03, 2016 at 08:59:46AM -0700, Eric Dumazet escreveu:
> From: Eric Dumazet <edumazet@google.com>
> 
> While fuzzing kernel with syzkaller, Andrey reported a nasty crash
> in inet6_bind() caused by DCCP lacking a required method.

Ouch, thanks, forgot the mapped case :-)

Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Fixes: ab1e0a13d7029 ("[SOCK] proto: Add hashinfo member to struct proto")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  net/dccp/ipv6.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index 3828f94b234c..95353bdbfa7b 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -956,6 +956,7 @@ static const struct inet_connection_sock_af_ops
> dccp_ipv6_mapped = {
>  	.getsockopt	   = ipv6_getsockopt,
>  	.addr2sockaddr	   = inet6_csk_addr2sockaddr,
>  	.sockaddr_len	   = sizeof(struct sockaddr_in6),
> +	.bind_conflict	   = inet6_csk_bind_conflict,
>  #ifdef CONFIG_COMPAT
>  	.compat_setsockopt = compat_ipv6_setsockopt,
>  	.compat_getsockopt = compat_ipv6_getsockopt,
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dccp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply
* stmmac: GMAC_RGSMIIIS reports bogus values
From: Alexey Brodkin @ 2016-11-03 16:17 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: manabian@gmail.com, linux-kernel@vger.kernel.org,
	peppe.cavallaro@st.com, fabrice.gasnier@st.com,
	linux-snps-arc@lists.infradead.org, alexandre.torgue@gmail.com,
	preid@electromag.com.au, davem@davemloft.net

Hello,

I'm seeing pretty strange issue with GMAC reporting a lot of link state changes
based on bits in GMAC_RGSMIIIS. It looks like that:
-------------------------->8-----------------------
Link is Down
Link is Up - 10/Full
Link is Down
Link is Up - 10/Half
Link is Down
Link is Down
Link is Up - 10/Half
Link is Up -
1000/Half
Link is Down
Link is Down
Link is Down
Link is Down
Link is Up - 10/Half
Link is Down
Link is Down
Link is Up -
1000/Half
Link is Up - 1000/Full
-------------------------->8-----------------------

What's especially interesting my board with GMAC is connected to 100Mbit device
which means there's no chance for 1Gb mode to be set.

Also this has nothing to do with link state detected and reported by PHY via MDIO.
So obviously GMAC_RGSMIIIS bits are wrong. But given the fact that GMAC_RGSMIIIS bits
are set based on state of RXD[3:0] lines of RGMII I may only thing that it's
PHY (in my case DP83865) who's sending random data on the RXD during inter-frame gap.

Note data transferred through that networking connection is perfectly correct and
actually I haven't see those link state prints before kernel v4.8 basically
because the prints in question were implemented with pr_debug() and then with [1]
we got pr_info() that made prints visible by default.

Since I don't have any means to capture all required GMII signals to do better
analysis and my data is not corrupted in the end I'm thinking about way how to
mute these pretty senseless messages.

One thing I may think of we may disable checking of GMAC_RGSMIIIS if a particular
board uses MDIO for PHY setup. Something like that:
-------------------------->8-----------------------
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -337,7 +337,7 @@ static int dwmac1000_irq_status(struct mac_device_info *hw,
 
        dwmac_pcs_isr(ioaddr, GMAC_PCS_BASE, intr_status, x);
 
-       if (intr_status & PCS_RGSMIIIS_IRQ)
+       if (!priv->use_mdio && (intr_status & PCS_RGSMIIIS_IRQ))
                dwmac1000_rgsmii(ioaddr, x);
 
        return ret;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6c85b61aaa0b..34e9de0450ba 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3356,11 +3356,13 @@ int stmmac_dvr_probe(struct device *device,
 
        stmmac_check_pcs_mode(priv);
 
+       priv->use_mdio = 0;
        if (priv->hw->pcs != STMMAC_PCS_RGMII  &&
            priv->hw->pcs != STMMAC_PCS_TBI &&
            priv->hw->pcs != STMMAC_PCS_RTBI) {
                /* MDIO bus Registration */
                ret = stmmac_mdio_register(ndev);
+               priv->use_mdio = 1;
                if (ret < 0) {
                        pr_debug("%s: MDIO bus (id: %d) registration failed",
                                 __func__, priv->plat->bus_id);
-------------------------->8-----------------------

Any thoughts on that are much appreciated!

Regards,
Alexey

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=70523e639bf8ca09b3357371c3546cee55c06351

^ permalink raw reply related
* [PATCH net] ipv6: dccp: add missing bind_conflict to dccp_ipv6_mapped
From: Eric Dumazet @ 2016-11-03 15:59 UTC (permalink / raw)
  To: David Miller
  Cc: Andrey Konovalov, netdev, syzkaller, Gerrit Renker, dccp,
	Arnaldo Carvalho de Melo
In-Reply-To: <CANn89i+YLvnFZFQTQyUA_jxYtZ1RE1NCCb3F4gqdia+w8wuK8A@mail.gmail.com>

From: Eric Dumazet <edumazet@google.com>

While fuzzing kernel with syzkaller, Andrey reported a nasty crash
in inet6_bind() caused by DCCP lacking a required method.

Fixes: ab1e0a13d7029 ("[SOCK] proto: Add hashinfo member to struct proto")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 net/dccp/ipv6.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 3828f94b234c..95353bdbfa7b 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -956,6 +956,7 @@ static const struct inet_connection_sock_af_ops
dccp_ipv6_mapped = {
 	.getsockopt	   = ipv6_getsockopt,
 	.addr2sockaddr	   = inet6_csk_addr2sockaddr,
 	.sockaddr_len	   = sizeof(struct sockaddr_in6),
+	.bind_conflict	   = inet6_csk_bind_conflict,
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_ipv6_setsockopt,
 	.compat_getsockopt = compat_ipv6_getsockopt,

^ permalink raw reply related
* Re: [Intel-wired-lan] [PATCH] e1000e: free IRQ when the link is up or down
From: Baicar, Tyler @ 2016-11-03 15:54 UTC (permalink / raw)
  To: Ruinskiy, Dima, Kirsher, Jeffrey T,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, okaya@codeaurora.org,
	timur@codeaurora.org
In-Reply-To: <36CDDD56DDB4D44E911123902EFC26B06CD48342@HASMSX110.ger.corp.intel.com>

On 11/3/2016 2:09 AM, Ruinskiy, Dima wrote:
>> -----Original Message-----
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
>> Behalf Of Tyler Baicar
>> Sent: Wednesday, 02 November, 2016 23:08
>> To: Kirsher, Jeffrey T; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>> okaya@codeaurora.org; timur@codeaurora.org
>> Cc: Tyler Baicar
>> Subject: [Intel-wired-lan] [PATCH] e1000e: free IRQ when the link is up or
>> down
>>
>> Move IRQ free code so that it will happen regardless of the link state.
>> Currently the e1000e driver only releases its IRQ if the link is up. This is not
>> sufficient because it is possible for a link to go down without releasing the IRQ.
>> A secondary bus reset can cause this case to happen.
>>
>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
>> ---
>> drivers/net/ethernet/intel/e1000e/netdev.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
>> b/drivers/net/ethernet/intel/e1000e/netdev.c
>> index 7017281..36cfcb0 100644
>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>> @@ -4679,12 +4679,13 @@ int e1000e_close(struct net_device *netdev)
>>
>> 	if (!test_bit(__E1000_DOWN, &adapter->state)) {
>> 		e1000e_down(adapter, true);
>> -		e1000_free_irq(adapter);
>>
>> 		/* Link status message must follow this format */
>> 		pr_info("%s NIC Link is Down\n", adapter->netdev->name);
>> 	}
>>
>> +	e1000_free_irq(adapter);
>> +
>> 	napi_disable(&adapter->napi);
>>
>> 	e1000e_free_tx_resources(adapter->tx_ring);
> This is not correct. __E1000_DOWN has nothing to do with link state. It is an internal driver status bit that indicates that device shutdown is in progress.
>
> I would not change this code without checking very carefully the driver state machine. This can cause a whole lot of issues. Did you encounter some particular problem that is resolved by this change?
Hello Dima,

The issue is that when a secondary bus reset occurs the current code 
will not free the IRQ due to this __E1000_DOWN check. If the IRQ isn't 
freed, then later in e1000_remove we run into a kernel bug:

pcieport 0004:00:00.0: PCIe Bus Error: severity=Corrected, type=Physical 
Layer, id=0000(Receiver ID)
pcieport 0004:00:00.0:   device [17cb:0400] error 
status/mask=00000001/00006000
pcieport 0004:00:00.0:    [ 0] Receiver Error         (First)
pcieport 0004:00:00.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), 
type=Transaction Layer, id=0000(Requester ID)
pcieport 0004:00:00.0:   device [17cb:0400] error 
status/mask=00004000/00400000
pcieport 0004:00:00.0:    [14] Completion Timeout     (First)
ACPI: \_SB_.PCI4: Device has suffered a power fault
kernel BUG at drivers/pci/msi.c:369!

The stack dump is:

free_msi_irqs+0x6c/0x1a8
pci_disable_msi+0xb0/0x148
e1000e_reset_interrupt_capability+0x60/0x78
e1000_remove+0xc8/0x180
pci_device_remove+0x48/0x118
__device_release_driver+0x80/0x108
device_release_driver+0x2c/0x40
pci_stop_bus_device+0xa0/0xb0
pci_stop_bus_device+0x3c/0xb0
pci_stop_root_bus+0x54/0x80
acpi_pci_root_remove+0x28/0x64
acpi_bus_trim+0x6c/0xa4
acpi_device_hotplug+0x19c/0x3f4
acpi_hotplug_work_fn+0x28/0x3c
process_one_work+0x150/0x460
worker_thread+0x50/0x4b8
kthread+0xd4/0xe8
ret_from_fork+0x10/0x50

This bug is hit because the IRQ still has action since it was never 
freed. This patch resolves this issue.

Thanks,
Tyler

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply
* URGENTLY CONFIRM
From: REV.JOHN GAZDA @ 2016-11-03 15:52 UTC (permalink / raw)


-- 
 HELLO,
 HAVE YOU RECEIVED THE FUNDS? PLEASE CONFIRM TO ME IF YOU HAVE RECEIVED
 THE FUND OR NOT.
 REV.JOHN GAZDA

^ permalink raw reply
* Re: net/ipv6: null-ptr-deref in inet6_bind
From: Andrey Konovalov @ 2016-11-03 15:49 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Dmitry Vyukov,
	Alexander Potapenko, Kostya Serebryany, syzkaller
In-Reply-To: <CANn89i+YLvnFZFQTQyUA_jxYtZ1RE1NCCb3F4gqdia+w8wuK8A@mail.gmail.com>

Hi Eric,

It seems that your patch fixes the issue, I'm not seeing the report any more.

Tested-by: Andrey Konovalov <andreyknvl@google.com>

Thanks!

On Thu, Nov 3, 2016 at 4:39 PM, Eric Dumazet <edumazet@google.com> wrote:
> On Wed, Nov 2, 2016 at 2:14 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
>> Hi,
>>
>> I've got the following error report while running the syzkaller fuzzer:
>>
>> BUG: unable to handle kernel NULL pointer dereference at           (null)
>> IP: [<          (null)>]           (null)
>> PGD 66b6f067 [  102.549865] PUD 66c6e067
>> PMD 0 [  102.549865]
>> Oops: 0010 [#1] SMP KASAN
>> Modules linked in:
>> CPU: 0 PID: 4143 Comm: a.out Not tainted 4.9.0-rc3+ #336
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> task: ffff880066b1c200 task.stack: ffff880065b58000
>> RIP: 0010:[<0000000000000000>]  [<          (null)>]           (null)
>> RSP: 0018:ffff880065b5fbc0  EFLAGS: 00010246
>> RAX: ffff880066b1c200 RBX: ffff88006873864a RCX: 0000000000000000
>> RDX: 0000000000000001 RSI: ffff880068738640 RDI: ffff880063bd3200
>> RBP: ffff880065b5fd20 R08: 1ffff1000c77a713 R09: dffffc0000000000
>> R10: ffffffff844fc800 R11: 1ffff1000d0e70c9 R12: ffffffff84e7e040
>> R13: ffff880068738640 R14: ffff880063bd3200 R15: ffffffff86836380
>> FS:  00007f40b7acf700(0000) GS:ffff88006cc00000(0000) knlGS:0000000000000000
>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 0000000000000000 CR3: 000000006bb28000 CR4: 00000000000006f0
>> Stack:
>>  ffffffff83099988 ffffffff8479f7e8 ffffffff81208580 1ffff1000000000c
>>  0000000041b58ab3 ffffffff8479f7e8 ffffffff81208580 ffffffff812506ed
>>  0000000000000007 ffff880065b5fc18 ffffffff812506ed ffff880065b5fcd0
>> Call Trace:
>>  [<ffffffff832cf4fc>] inet6_bind+0x8ec/0x1020 net/ipv6/af_inet6.c:384
>>  [<ffffffff82b7033c>] SYSC_bind+0x1ec/0x250 net/socket.c:1367
>>  [<ffffffff82b72ae4>] SyS_bind+0x24/0x30 net/socket.c:1353
>>  [<ffffffff83fc0401>] entry_SYSCALL_64_fastpath+0x1f/0xc2
>> arch/x86/entry/entry_64.S:209
>> Code:  Bad RIP value.
>> RIP  [<          (null)>]           (null)
>>  RSP <ffff880065b5fbc0>
>> CR2: 0000000000000000
>> ---[ end trace b5ec698ae4926a97 ]---
>> Kernel panic - not syncing: Fatal exception in interrupt
>> Kernel Offset: disabled
>> ---[ end Kernel panic - not syncing: Fatal exception in interrupt
>>
>> On commit 0c183d92b20b5c84ca655b45ef57b3318b83eb9e (Oct 31).
>>
>> I'm able to reproduce it with the attached program by running it as:
>> $ gcc -lpthread inet6-bind-poc.c
>> $ while true; do ./a.out; done
>>
>> Thanks!
>
> Looks like this patch should fix it ?
>
> Thanks !
>
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index 3828f94b234c..95353bdbfa7b 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -956,6 +956,7 @@ static const struct inet_connection_sock_af_ops
> dccp_ipv6_mapped = {
>         .getsockopt        = ipv6_getsockopt,
>         .addr2sockaddr     = inet6_csk_addr2sockaddr,
>         .sockaddr_len      = sizeof(struct sockaddr_in6),
> +       .bind_conflict     = inet6_csk_bind_conflict,
>  #ifdef CONFIG_COMPAT
>         .compat_setsockopt = compat_ipv6_setsockopt,
>         .compat_getsockopt = compat_ipv6_getsockopt,

^ permalink raw reply
* Re: [RFC] make kmemleak scan __ro_after_init section (was: Re: [PATCH 0/5] genetlink improvements)
From: Johannes Berg @ 2016-11-03 15:48 UTC (permalink / raw)
  To: Jakub Kicinski, Cong Wang
  Cc: Linux Kernel Network Developers, LKML, Catalin Marinas, linux-mm
In-Reply-To: <20161102234755.4381f528@jkicinski-Precision-T1700>

Hi,

Sorry for not chipping in earlier - LPC is taking my time.

> > > > Looks like we are missing a kfree(family->attrbuf); on error
> > > > path, but it is not related to Johannes' recent patches.

Actually, I think it *is* related to my patch - I inserted the code
there in the wrong place or so. I'll find a fix for that when I'm back
home, or you (Cong) can submit yours. It wasn't likely that this was
the problem though, since that's just an error path that should never
happen (we have <30 genl families, and a 16-bit space for their IDs)

> I realized that kmemleak is not scanning the __ro_after_init
> section...
> Following patch solves the false positives but I wonder if it's the
> right/acceptable solution.

Hah, makes sense to me, but I guess we really want Catalin to comment
:)

johannes

^ 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