Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/5] ip6_gre: Fix MTU setting
From: Tom Herbert @ 2016-05-10  0:12 UTC (permalink / raw)
  To: davem, netdev, alexander.duyck; +Cc: kernel-team
In-Reply-To: <1462839132-980991-1-git-send-email-tom@herbertland.com>

In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
configuration. For hard_header_len and mtu calculation include
IPv6 header and encapsulation overhead.

In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
the configuration. Revert to setting hard_header_len instead of
needed_headroom.

Tested:

./ip link add name tun8 type ip6gretap remote \
2401:db00:20:911a:face:0:27:0 local \
2401:db00:20:911a:face:0:25:0 ttl 225

Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.

./ip link add name tun8 type ip6gretap remote \
2401:db00:20:911a:face:0:27:0 local \
2401:db00:20:911a:face:0:25:0 ttl 225 okey 123

Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.

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

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 47b671a..6d0aa94 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -700,7 +700,7 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
 	struct net_device *dev = t->dev;
 	struct __ip6_tnl_parm *p = &t->parms;
 	struct flowi6 *fl6 = &t->fl.u.ip6;
-	int addend = sizeof(struct ipv6hdr) + 4;
+	int t_hlen;
 
 	if (dev->type != ARPHRD_ETHER) {
 		memcpy(dev->dev_addr, &p->laddr, sizeof(struct in6_addr));
@@ -727,16 +727,11 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
 	else
 		dev->flags &= ~IFF_POINTOPOINT;
 
-	/* Precalculate GRE options length */
-	if (t->parms.o_flags&(GRE_CSUM|GRE_KEY|GRE_SEQ)) {
-		if (t->parms.o_flags&GRE_CSUM)
-			addend += 4;
-		if (t->parms.o_flags&GRE_KEY)
-			addend += 4;
-		if (t->parms.o_flags&GRE_SEQ)
-			addend += 4;
-	}
-	t->hlen = addend;
+	t->tun_hlen = gre_calc_hlen(t->parms.o_flags);
+
+	t->hlen = t->tun_hlen;
+
+	t_hlen = t->hlen + sizeof(struct ipv6hdr);
 
 	if (p->flags & IP6_TNL_F_CAP_XMIT) {
 		int strict = (ipv6_addr_type(&p->raddr) &
@@ -750,10 +745,11 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
 			return;
 
 		if (rt->dst.dev) {
-			dev->hard_header_len = rt->dst.dev->hard_header_len + addend;
+			dev->hard_header_len = rt->dst.dev->hard_header_len +
+					       t_hlen;
 
 			if (set_mtu) {
-				dev->mtu = rt->dst.dev->mtu - addend;
+				dev->mtu = rt->dst.dev->mtu - t_hlen;
 				if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
 					dev->mtu -= 8;
 				if (dev->type == ARPHRD_ETHER)
@@ -1027,11 +1023,12 @@ static int ip6gre_tunnel_init_common(struct net_device *dev)
 
 	tunnel->tun_hlen = gre_calc_hlen(tunnel->parms.o_flags);
 
-	t_hlen = tunnel->hlen + sizeof(struct ipv6hdr);
+	tunnel->hlen = tunnel->tun_hlen;
 
-	dev->needed_headroom	= LL_MAX_HEADER + t_hlen + 4;
-	dev->mtu		= ETH_DATA_LEN - t_hlen - 4;
+	t_hlen = tunnel->hlen + sizeof(struct ipv6hdr);
 
+	dev->hard_header_len = LL_MAX_HEADER + t_hlen;
+	dev->mtu = ETH_DATA_LEN - t_hlen;
 	if (!(tunnel->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
 		dev->mtu -= 8;
 
-- 
2.8.0.rc2

^ permalink raw reply related

* [PATCH net-next 0/5] ip6: Transmit tunneling fixes
From: Tom Herbert @ 2016-05-10  0:12 UTC (permalink / raw)
  To: davem, netdev, alexander.duyck; +Cc: kernel-team

Several fixes suggested by Alexander.

Tested: Running netperf TCP_STREAM with gretap and keyid configured.
Visually verified that MTU is correctly being set. Did not test HW
offload (Alexander plese try)


Tom Herbert (5):
  ip6_gre: Fix MTU setting
  gre6: Fix flag translations
  ip6_gre: Set inner protocol correctly in __gre6_xmit
  ip6: Don't set transport header in IPv6 tunneling
  ip6_gre: Use correct flags for reading TUNNEL_SEQ

 net/ipv6/ip6_gre.c    | 55 ++++++++++++++++++++++++++-------------------------
 net/ipv6/ip6_tunnel.c |  2 --
 2 files changed, 28 insertions(+), 29 deletions(-)

-- 
2.8.0.rc2

^ permalink raw reply

* [PATCH 5/6] drivers: net: xgene: Using static MSS values
From: Iyappan Subramanian @ 2016-05-10  0:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-arm-kernel, patches, toanle, Iyappan Subramanian
In-Reply-To: <1462838656-22043-1-git-send-email-isubramanian@apm.com>

Due to the nature of hardware design for TSO, if the MSS values that are
stored in the register, changes during TSO operation, data corruption may
occur.

This patch fixes the issue by using one of the predefined MSS values.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Tested-by: Toan Le <toanle@apm.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c    |  2 -
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h    |  2 +
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c  | 45 ++++++++++++++++++++++-
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |  6 +--
 drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c | 20 ++++++++--
 5 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 0050878..2f5638f 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -219,8 +219,6 @@ void xgene_enet_parse_error(struct xgene_enet_desc_ring *ring,
 			    struct xgene_enet_pdata *pdata,
 			    enum xgene_enet_err_code status)
 {
-	struct rtnl_link_stats64 *stats = &pdata->stats;
-
 	switch (status) {
 	case INGRESS_CRC:
 		ring->rx_crc_errors++;
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
index ecfeffe..a3c12dc 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
@@ -227,6 +227,8 @@ enum xgene_enet_rm {
 #define TCPHDR_LEN			6
 #define IPHDR_POS			6
 #define IPHDR_LEN			6
+#define MSS_POS				20
+#define MSS_LEN				2
 #define EC_POS				22	/* Enable checksum */
 #define EC_LEN				1
 #define ET_POS				23	/* Enable TSO */
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index d992ae8..af272cb 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -179,6 +179,46 @@ static int xgene_enet_tx_completion(struct xgene_enet_desc_ring *cp_ring,
 	return ret;
 }
 
+static void apm_enet_init_mss_values(struct net_device *ndev)
+{
+	struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+	int i;
+
+	pdata->mss_sw[0] = 64;
+	pdata->mss_sw[1] = 1024;
+	pdata->mss_sw[2] = 1400;
+	pdata->mss_sw[3] = 1446;
+
+	for (i = 0; i < NUM_MSS_REG; i++)
+		pdata->mac_ops->set_mss(pdata, pdata->mss_sw[i], i);
+}
+
+static int apm_enet_get_mss_index(struct net_device *ndev, u32 new_mss)
+{
+	struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+	int i;
+
+	for (i = NUM_MSS_REG - 1; i >= 0; i--) {
+		if (new_mss >= pdata->mss_sw[i])
+			return i;
+	}
+
+	return -1;
+}
+
+static int xgene_enet_setup_mss(struct net_device *ndev, u64 *hopinfo, u32 mss)
+{
+	int mss_index;
+
+	mss_index = apm_enet_get_mss_index(ndev, mss);
+	if (mss_index < 0)
+		return mss_index;
+
+	*hopinfo |= SET_VAL(MSS, mss_index);
+
+	return 0;
+}
+
 static u64 xgene_enet_work_msg(struct sk_buff *skb)
 {
 	struct net_device *ndev = skb->dev;
@@ -227,6 +267,9 @@ static u64 xgene_enet_work_msg(struct sk_buff *skb)
 			if (!mss || ((skb->len - hdr_len) <= mss))
 				goto out;
 
+			if (xgene_enet_setup_mss(ndev, &hopinfo, mss))
+				return 0;
+
 			hopinfo |= SET_BIT(ET);
 		}
 	} else if (iph->protocol == IPPROTO_UDP) {
@@ -1625,7 +1668,7 @@ static int xgene_enet_probe(struct platform_device *pdev)
 
 	if (pdata->phy_mode == PHY_INTERFACE_MODE_XGMII) {
 		ndev->features |= NETIF_F_TSO;
-		pdata->mss = XGENE_ENET_MSS;
+		apm_enet_init_mss_values(ndev);
 	}
 	ndev->hw_features = ndev->features;
 
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
index 092fbec..20a8b59 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
@@ -46,7 +46,7 @@
 #define NUM_PKT_BUF	64
 #define NUM_BUFPOOL	32
 #define MAX_EXP_BUFFS	256
-#define XGENE_ENET_MSS	1448
+#define NUM_MSS_REG	4
 #define XGENE_MIN_ENET_FRAME_SIZE	60
 
 #define XGENE_MAX_ENET_IRQ	16
@@ -141,7 +141,7 @@ struct xgene_mac_ops {
 	void (*tx_disable)(struct xgene_enet_pdata *pdata);
 	void (*rx_disable)(struct xgene_enet_pdata *pdata);
 	void (*set_mac_addr)(struct xgene_enet_pdata *pdata);
-	void (*set_mss)(struct xgene_enet_pdata *pdata);
+	void (*set_mss)(struct xgene_enet_pdata *pdata, u16 mss, u8 index);
 	void (*link_state)(struct work_struct *work);
 };
 
@@ -208,7 +208,7 @@ struct xgene_enet_pdata {
 	u8 eth_bufnum;
 	u8 bp_bufnum;
 	u16 ring_num;
-	u32 mss;
+	u32 mss_sw[NUM_MSS_REG];
 	u8 tx_delay;
 	u8 rx_delay;
 };
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
index ba030dc..cb9f681 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c
@@ -184,9 +184,24 @@ static void xgene_xgmac_set_mac_addr(struct xgene_enet_pdata *pdata)
 	xgene_enet_wr_mac(pdata, HSTMACADR_MSW_ADDR, addr1);
 }
 
-static void xgene_xgmac_set_mss(struct xgene_enet_pdata *pdata)
+static void xgene_xgmac_set_mss(struct xgene_enet_pdata *pdata, u16 mss,
+				u8 index)
 {
-	xgene_enet_wr_csr(pdata, XG_TSIF_MSS_REG0_ADDR, pdata->mss);
+	bool reg_index;
+	u32 data;
+
+	xgene_enet_rd_csr(pdata, XG_TSIF_MSS_REG0_ADDR, &data);
+	reg_index = (index < 2) ? 0 : 1;
+
+	if (!(index & 0x1)) {
+		data &= 0xffff0000;
+		data |= mss;
+	} else {
+		data &= 0xffff;
+		data |= (mss << 16);
+	}
+
+	xgene_enet_wr_csr(pdata, XG_TSIF_MSS_REG0_ADDR + reg_index * 4, data);
 }
 
 static u32 xgene_enet_link_status(struct xgene_enet_pdata *pdata)
@@ -210,7 +225,6 @@ static void xgene_xgmac_init(struct xgene_enet_pdata *pdata)
 	xgene_enet_wr_mac(pdata, AXGMAC_CONFIG_1, data);
 
 	xgene_xgmac_set_mac_addr(pdata);
-	xgene_xgmac_set_mss(pdata);
 
 	xgene_enet_rd_csr(pdata, XG_RSIF_CONFIG_REG_ADDR, &data);
 	data |= CFG_RSIF_FPBUFF_TIMEOUT_EN;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 6/6] drivers: net: xgene: fix register offset
From: Iyappan Subramanian @ 2016-05-10  0:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-arm-kernel, patches, toanle, Iyappan Subramanian
In-Reply-To: <1462838656-22043-1-git-send-email-isubramanian@apm.com>

This patch fixes SG_RX_DV_GATE_REG_0_ADDR register offset
and ring state field lengths.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Tested-by: Toan Le <toanle@apm.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h    | 6 +++---
 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
index a3c12dc..a4a1d3e 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
@@ -86,7 +86,7 @@ enum xgene_enet_rm {
 #define RINGADDRL_POS		5
 #define RINGADDRL_LEN		27
 #define RINGADDRH_POS		0
-#define RINGADDRH_LEN		6
+#define RINGADDRH_LEN		7
 #define RINGSIZE_POS		23
 #define RINGSIZE_LEN		3
 #define RINGTYPE_POS		19
@@ -94,9 +94,9 @@ enum xgene_enet_rm {
 #define RINGMODE_POS		20
 #define RINGMODE_LEN		3
 #define RECOMTIMEOUTL_POS	28
-#define RECOMTIMEOUTL_LEN	3
+#define RECOMTIMEOUTL_LEN	4
 #define RECOMTIMEOUTH_POS	0
-#define RECOMTIMEOUTH_LEN	2
+#define RECOMTIMEOUTH_LEN	3
 #define NUMMSGSINQ_POS		1
 #define NUMMSGSINQ_LEN		16
 #define ACCEPTLERR		BIT(19)
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
index 29a71b4..002df5a 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h
@@ -33,7 +33,7 @@
 #define LINK_STATUS			BIT(2)
 #define LINK_UP				BIT(15)
 #define MPA_IDLE_WITH_QMI_EMPTY		BIT(12)
-#define SG_RX_DV_GATE_REG_0_ADDR	0x0dfc
+#define SG_RX_DV_GATE_REG_0_ADDR	0x05fc
 
 extern const struct xgene_mac_ops xgene_sgmac_ops;
 extern const struct xgene_port_ops xgene_sgport_ops;
-- 
1.9.1

^ permalink raw reply related

* [PATCH 4/6] drivers: net: xgene: fix statistics counters race condition
From: Iyappan Subramanian @ 2016-05-10  0:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-arm-kernel, patches, toanle, Iyappan Subramanian
In-Reply-To: <1462838656-22043-1-git-send-email-isubramanian@apm.com>

This patch fixes the race condition on updating the statistics
counters by moving the counters to the ring structure.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Tested-by: Toan Le <toanle@apm.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c   | 17 +++++++----
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h   |  2 ++
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 39 ++++++++++++++++++------
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h | 10 ++++++
 4 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 457f745..0050878 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -223,23 +223,28 @@ void xgene_enet_parse_error(struct xgene_enet_desc_ring *ring,
 
 	switch (status) {
 	case INGRESS_CRC:
-		stats->rx_crc_errors++;
+		ring->rx_crc_errors++;
+		ring->rx_dropped++;
 		break;
 	case INGRESS_CHECKSUM:
 	case INGRESS_CHECKSUM_COMPUTE:
-		stats->rx_errors++;
+		ring->rx_errors++;
+		ring->rx_dropped++;
 		break;
 	case INGRESS_TRUNC_FRAME:
-		stats->rx_frame_errors++;
+		ring->rx_frame_errors++;
+		ring->rx_dropped++;
 		break;
 	case INGRESS_PKT_LEN:
-		stats->rx_length_errors++;
+		ring->rx_length_errors++;
+		ring->rx_dropped++;
 		break;
 	case INGRESS_PKT_UNDER:
-		stats->rx_frame_errors++;
+		ring->rx_frame_errors++;
+		ring->rx_dropped++;
 		break;
 	case INGRESS_FIFO_OVERRUN:
-		stats->rx_fifo_errors++;
+		ring->rx_fifo_errors++;
 		break;
 	default:
 		break;
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
index ba7da98..ecfeffe 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
@@ -201,6 +201,8 @@ enum xgene_enet_rm {
 #define USERINFO_LEN			32
 #define FPQNUM_POS			32
 #define FPQNUM_LEN			12
+#define ELERR_POS                       46
+#define ELERR_LEN                       2
 #define NV_POS				50
 #define NV_LEN				1
 #define LL_POS				51
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index e75d1a9..d992ae8 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -443,8 +443,8 @@ static netdev_tx_t xgene_enet_start_xmit(struct sk_buff *skb,
 
 	skb_tx_timestamp(skb);
 
-	pdata->stats.tx_packets++;
-	pdata->stats.tx_bytes += skb->len;
+	tx_ring->tx_packets++;
+	tx_ring->tx_bytes += skb->len;
 
 	pdata->ring_ops->wr_cmd(tx_ring, count);
 	return NETDEV_TX_OK;
@@ -483,12 +483,12 @@ static int xgene_enet_rx_frame(struct xgene_enet_desc_ring *rx_ring,
 	skb = buf_pool->rx_skb[skb_index];
 
 	/* checking for error */
-	status = GET_VAL(LERR, le64_to_cpu(raw_desc->m0));
+	status = (GET_VAL(ELERR, le64_to_cpu(raw_desc->m0)) << LERR_LEN) ||
+		  GET_VAL(LERR, le64_to_cpu(raw_desc->m0));
 	if (unlikely(status > 2)) {
 		dev_kfree_skb_any(skb);
 		xgene_enet_parse_error(rx_ring, netdev_priv(rx_ring->ndev),
 				       status);
-		pdata->stats.rx_dropped++;
 		ret = -EIO;
 		goto out;
 	}
@@ -506,8 +506,8 @@ static int xgene_enet_rx_frame(struct xgene_enet_desc_ring *rx_ring,
 		xgene_enet_skip_csum(skb);
 	}
 
-	pdata->stats.rx_packets++;
-	pdata->stats.rx_bytes += datalen;
+	rx_ring->rx_packets++;
+	rx_ring->rx_bytes += datalen;
 	napi_gro_receive(&rx_ring->napi, skb);
 out:
 	if (--rx_ring->nbufpool == 0) {
@@ -1127,11 +1127,30 @@ static struct rtnl_link_stats64 *xgene_enet_get_stats64(
 {
 	struct xgene_enet_pdata *pdata = netdev_priv(ndev);
 	struct rtnl_link_stats64 *stats = &pdata->stats;
+	struct xgene_enet_desc_ring *ring;
+	int i;
+
+	memset(stats, 0, sizeof(struct rtnl_link_stats64));
+	for (i = 0; i < pdata->txq_cnt; i++) {
+		ring = pdata->tx_ring[i];
+		if (ring) {
+			stats->tx_packets += ring->tx_packets;
+			stats->tx_bytes += ring->tx_bytes;
+		}
+	}
 
-	stats->rx_errors += stats->rx_length_errors +
-			    stats->rx_crc_errors +
-			    stats->rx_frame_errors +
-			    stats->rx_fifo_errors;
+	for (i = 0; i < pdata->rxq_cnt; i++) {
+		ring = pdata->rx_ring[i];
+		if (ring) {
+			stats->rx_packets += ring->rx_packets;
+			stats->rx_bytes += ring->rx_bytes;
+			stats->rx_errors += ring->rx_length_errors +
+				ring->rx_crc_errors +
+				ring->rx_frame_errors +
+				ring->rx_fifo_errors;
+			stats->rx_dropped += ring->rx_dropped;
+		}
+	}
 	memcpy(storage, &pdata->stats, sizeof(struct rtnl_link_stats64));
 
 	return storage;
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
index cc40c30..092fbec 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
@@ -121,6 +121,16 @@ struct xgene_enet_desc_ring {
 		struct xgene_enet_raw_desc16 *raw_desc16;
 	};
 	__le64 *exp_bufs;
+	u64 tx_packets;
+	u64 tx_bytes;
+	u64 rx_packets;
+	u64 rx_bytes;
+	u64 rx_dropped;
+	u64 rx_errors;
+	u64 rx_length_errors;
+	u64 rx_crc_errors;
+	u64 rx_frame_errors;
+	u64 rx_fifo_errors;
 };
 
 struct xgene_mac_ops {
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/6] drivers: net: xgene: fix ununiform latency across queues
From: Iyappan Subramanian @ 2016-05-10  0:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-arm-kernel, patches, toanle, Iyappan Subramanian
In-Reply-To: <1462838656-22043-1-git-send-email-isubramanian@apm.com>

This patch addresses ununiform latency across queues by adding
more queues to match with, upto number of CPU cores.

Also, number of interrupts are increased and the channel numbers
are reordered.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Tested-by: Toan Le <toanle@apm.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 30 ++++++++++++++++++------
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h |  8 +++----
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index ed90664..e75d1a9 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -1247,6 +1247,13 @@ static int xgene_enet_get_irqs(struct xgene_enet_pdata *pdata)
 	for (i = 0; i < max_irqs; i++) {
 		ret = platform_get_irq(pdev, i);
 		if (ret <= 0) {
+			if (pdata->phy_mode == PHY_INTERFACE_MODE_XGMII) {
+				max_irqs = i;
+				pdata->rxq_cnt = max_irqs / 2;
+				pdata->txq_cnt = max_irqs / 2;
+				pdata->cq_cnt = max_irqs / 2;
+				break;
+			}
 			dev_err(dev, "Unable to get ENET IRQ\n");
 			ret = ret ? : -ENXIO;
 			return ret;
@@ -1450,19 +1457,28 @@ static void xgene_enet_setup_ops(struct xgene_enet_pdata *pdata)
 		pdata->port_ops = &xgene_xgport_ops;
 		pdata->cle_ops = &xgene_cle3in_ops;
 		pdata->rm = RM0;
-		pdata->rxq_cnt = XGENE_NUM_RX_RING;
-		pdata->txq_cnt = XGENE_NUM_TX_RING;
-		pdata->cq_cnt = XGENE_NUM_TXC_RING;
+		if (!pdata->rxq_cnt) {
+			pdata->rxq_cnt = XGENE_NUM_RX_RING;
+			pdata->txq_cnt = XGENE_NUM_TX_RING;
+			pdata->cq_cnt = XGENE_NUM_TXC_RING;
+		}
 		break;
 	}
 
 	if (pdata->enet_id == XGENE_ENET1) {
 		switch (pdata->port_id) {
 		case 0:
-			pdata->cpu_bufnum = START_CPU_BUFNUM_0;
-			pdata->eth_bufnum = START_ETH_BUFNUM_0;
-			pdata->bp_bufnum = START_BP_BUFNUM_0;
-			pdata->ring_num = START_RING_NUM_0;
+			if (pdata->phy_mode == PHY_INTERFACE_MODE_XGMII) {
+				pdata->cpu_bufnum = X2_START_CPU_BUFNUM_0;
+				pdata->eth_bufnum = X2_START_ETH_BUFNUM_0;
+				pdata->bp_bufnum = X2_START_BP_BUFNUM_0;
+				pdata->ring_num = START_RING_NUM_0;
+			} else {
+				pdata->cpu_bufnum = START_CPU_BUFNUM_0;
+				pdata->eth_bufnum = START_ETH_BUFNUM_0;
+				pdata->bp_bufnum = START_BP_BUFNUM_0;
+				pdata->ring_num = START_RING_NUM_0;
+			}
 			break;
 		case 1:
 			if (pdata->phy_mode == PHY_INTERFACE_MODE_XGMII) {
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
index 0a2887b..cc40c30 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h
@@ -49,10 +49,10 @@
 #define XGENE_ENET_MSS	1448
 #define XGENE_MIN_ENET_FRAME_SIZE	60
 
-#define XGENE_MAX_ENET_IRQ	8
-#define XGENE_NUM_RX_RING	4
-#define XGENE_NUM_TX_RING	4
-#define XGENE_NUM_TXC_RING	4
+#define XGENE_MAX_ENET_IRQ	16
+#define XGENE_NUM_RX_RING	8
+#define XGENE_NUM_TX_RING	8
+#define XGENE_NUM_TXC_RING	8
 
 #define START_CPU_BUFNUM_0	0
 #define START_ETH_BUFNUM_0	2
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/6] drivers: net: xgene: fix sharing of irqs
From: Iyappan Subramanian @ 2016-05-10  0:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-arm-kernel, patches, toanle, Iyappan Subramanian
In-Reply-To: <1462838656-22043-1-git-send-email-isubramanian@apm.com>

Since hardware doesn't allow sharing of interrupts,
this patch fixes the same by removing IRQF_SHARED flag.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Tested-by: Toan Le <toanle@apm.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index aa87049..ed90664 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -630,7 +630,7 @@ static int xgene_enet_register_irq(struct net_device *ndev)
 		ring = pdata->rx_ring[i];
 		irq_set_status_flags(ring->irq, IRQ_DISABLE_UNLAZY);
 		ret = devm_request_irq(dev, ring->irq, xgene_enet_rx_irq,
-				       IRQF_SHARED, ring->irq_name, ring);
+				       0, ring->irq_name, ring);
 		if (ret) {
 			netdev_err(ndev, "Failed to request irq %s\n",
 				   ring->irq_name);
@@ -641,7 +641,7 @@ static int xgene_enet_register_irq(struct net_device *ndev)
 		ring = pdata->tx_ring[i]->cp_ring;
 		irq_set_status_flags(ring->irq, IRQ_DISABLE_UNLAZY);
 		ret = devm_request_irq(dev, ring->irq, xgene_enet_rx_irq,
-				       IRQF_SHARED, ring->irq_name, ring);
+				       0, ring->irq_name, ring);
 		if (ret) {
 			netdev_err(ndev, "Failed to request irq %s\n",
 				   ring->irq_name);
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/6] drivers: net: xgene: fix IPv4 forward crash
From: Iyappan Subramanian @ 2016-05-10  0:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-arm-kernel, patches, toanle, Iyappan Subramanian
In-Reply-To: <1462838656-22043-1-git-send-email-isubramanian@apm.com>

This patch fixes the crash observed during IPv4 forward test by
setting the drop field in the dbptr.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Tested-by: Toan Le <toanle@apm.com>
---
 drivers/net/ethernet/apm/xgene/xgene_enet_cle.c | 11 ++++++-----
 drivers/net/ethernet/apm/xgene/xgene_enet_cle.h |  2 ++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c
index 6479288..472c0fb 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c
@@ -43,6 +43,7 @@ static void xgene_cle_idt_to_hw(u32 dstqid, u32 fpsel,
 static void xgene_cle_dbptr_to_hw(struct xgene_enet_pdata *pdata,
 				  struct xgene_cle_dbptr *dbptr, u32 *buf)
 {
+	buf[0] = SET_VAL(CLE_DROP, dbptr->drop);
 	buf[4] = SET_VAL(CLE_FPSEL, dbptr->fpsel) |
 		 SET_VAL(CLE_DSTQIDL, dbptr->dstqid);
 
@@ -412,7 +413,7 @@ static int xgene_enet_cle_init(struct xgene_enet_pdata *pdata)
 			.branch = {
 				{
 					/* IPV4 */
-					.valid = 0,
+					.valid = 1,
 					.next_packet_pointer = 22,
 					.jump_bw = JMP_FW,
 					.jump_rel = JMP_ABS,
@@ -420,7 +421,7 @@ static int xgene_enet_cle_init(struct xgene_enet_pdata *pdata)
 					.next_node = PKT_PROT_NODE,
 					.next_branch = 0,
 					.data = 0x8,
-					.mask = 0xffff
+					.mask = 0x0
 				},
 				{
 					.valid = 0,
@@ -456,7 +457,7 @@ static int xgene_enet_cle_init(struct xgene_enet_pdata *pdata)
 					.next_node = RSS_IPV4_TCP_NODE,
 					.next_branch = 0,
 					.data = 0x0600,
-					.mask = 0xffff
+					.mask = 0x00ff
 				},
 				{
 					/* UDP */
@@ -468,7 +469,7 @@ static int xgene_enet_cle_init(struct xgene_enet_pdata *pdata)
 					.next_node = RSS_IPV4_UDP_NODE,
 					.next_branch = 0,
 					.data = 0x1100,
-					.mask = 0xffff
+					.mask = 0x00ff
 				},
 				{
 					.valid = 0,
@@ -642,7 +643,7 @@ static int xgene_enet_cle_init(struct xgene_enet_pdata *pdata)
 				{
 					/* TCP DST Port */
 					.valid = 0,
-					.next_packet_pointer = 256,
+					.next_packet_pointer = 258,
 					.jump_bw = JMP_FW,
 					.jump_rel = JMP_ABS,
 					.operation = EQT,
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.h b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.h
index 13e829a..33c5f6b 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.h
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.h
@@ -83,6 +83,8 @@
 #define CLE_TYPE_POS		0
 #define CLE_TYPE_LEN		2
 
+#define CLE_DROP_POS		28
+#define CLE_DROP_LEN		1
 #define CLE_DSTQIDL_POS		25
 #define CLE_DSTQIDL_LEN		7
 #define CLE_DSTQIDH_POS		0
-- 
1.9.1

^ permalink raw reply related

* [PATCH 0/6] drivers: net: xgene: Bug fixes
From: Iyappan Subramanian @ 2016-05-10  0:04 UTC (permalink / raw)
  To: davem, netdev; +Cc: linux-arm-kernel, patches, toanle, Iyappan Subramanian

This patch set addresses the following bug fixes that were found during testing.

  1. IPv4 forward test crash
    - drivers: net: xgene: fix IPv4 forward crash

  2. Sharing of irqs
    - drivers: net: xgene: fix sharing of irqs

  3. Ununiform latency across queues
    - drivers: net: xgene: fix ununiform latency across queues

  4. Fix statistics counters race condition
    - drivers: net: xgene: fix statistics counters race condition

  5. TSO data corruption
    - drivers: net: xgene: Using static MSS values

  6. Correcting register offset and field lengths
    - drivers: net: xgene: fix register offset

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
---

Iyappan Subramanian (6):
  drivers: net: xgene: fix IPv4 forward crash
  drivers: net: xgene: fix sharing of irqs
  drivers: net: xgene: fix ununiform latency across queues
  drivers: net: xgene: fix statistics counters race condition
  drivers: net: xgene: Using static MSS values
  drivers: net: xgene: fix register offset

 drivers/net/ethernet/apm/xgene/xgene_enet_cle.c   |  11 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_cle.h   |   2 +
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c    |  19 ++--
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h    |  10 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c  | 118 ++++++++++++++++++----
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |  24 +++--
 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h |   2 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c |  20 +++-
 8 files changed, 159 insertions(+), 47 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH net-next] ip6_gre: Fix MTU setting
From: Tom Herbert @ 2016-05-09 23:47 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <CAKgT0UcHky39E9Sxo7L_ZtOOx9uUNy5NdLa78C+oLeB2q21o+g@mail.gmail.com>

On Mon, May 9, 2016 at 4:10 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, May 9, 2016 at 3:49 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Mon, May 9, 2016 at 3:44 PM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
>>>> configuration. For hard_header_len and mtu calculation include
>>>> IPv6 header and encapsulation overhead.
>>>>
>>>> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
>>>> the configuration. Revert to setting hard_header_len instead of
>>>> needed_headroom.
>>>>
>>>> Tested:
>>>>
>>>> ./ip link add name tun8 type ip6gretap remote \
>>>> 2401:db00:20:911a:face:0:27:0 local \
>>>> 2401:db00:20:911a:face:0:25:0 ttl 225
>>>>
>>>> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>>>>
>>>> ./ip link add name tun8 type ip6gretap remote \
>>>> 2401:db00:20:911a:face:0:27:0 local \
>>>> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>>>>
>>>> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>>>>
>>>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>>>
>>> So this gives me the correct result now.  However we still need patch
>>> 3 / 11 from the set you submitted next week in order to be able to
>>> even transmit because the flags are otherwise mangled.
>>>
>>> Tested-by: Alexander Duyck <aduyck@mirantis.com>
>>
>> Okay, I'll repost the flags patch as a standalone. Do you have any
>> other issues then in this path?
>
> This piece no.
>
> Like I said in the other email I did find two other issues.  First in
> __gre6_xmit I think you want to set the inner protocol to "protocol",
> not "proto".  Second is the issue with ip6_tnl_xmit stomping on
> skb->transport_header.  The second one is the big one that is blocking
> things badly right now.  That is the one that keeps me from doing
> anything with any sort of offload as the headers are jumping around
> out of order with the outer transport header having an offset that is
> further in than the inner mac header.
>
You mean "skb->transport_header = skb->network_header;"? It doesn't
look right to me either, but that line has been around a while....

> I'd be fine with you submitting the whole set as one piece with the
> other 11 patches you had just as long as we get those core pieces
> fixed in the patch set.
>
Probably just as easy to break it up.

> - Alex

^ permalink raw reply

* Re: [PATCH net-next] ip6_gre: Fix MTU setting
From: Alexander Duyck @ 2016-05-09 23:14 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <CAKgT0UcHky39E9Sxo7L_ZtOOx9uUNy5NdLa78C+oLeB2q21o+g@mail.gmail.com>

On Mon, May 9, 2016 at 4:10 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, May 9, 2016 at 3:49 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Mon, May 9, 2016 at 3:44 PM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
>>>> configuration. For hard_header_len and mtu calculation include
>>>> IPv6 header and encapsulation overhead.
>>>>
>>>> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
>>>> the configuration. Revert to setting hard_header_len instead of
>>>> needed_headroom.
>>>>
>>>> Tested:
>>>>
>>>> ./ip link add name tun8 type ip6gretap remote \
>>>> 2401:db00:20:911a:face:0:27:0 local \
>>>> 2401:db00:20:911a:face:0:25:0 ttl 225
>>>>
>>>> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>>>>
>>>> ./ip link add name tun8 type ip6gretap remote \
>>>> 2401:db00:20:911a:face:0:27:0 local \
>>>> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>>>>
>>>> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>>>>
>>>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>>>
>>> So this gives me the correct result now.  However we still need patch
>>> 3 / 11 from the set you submitted next week in order to be able to
>>> even transmit because the flags are otherwise mangled.
>>>
>>> Tested-by: Alexander Duyck <aduyck@mirantis.com>
>>
>> Okay, I'll repost the flags patch as a standalone. Do you have any
>> other issues then in this path?
>
> This piece no.

Actually I just realized there are two other bits that need to be updated.

I just copied/pasted the below out of a text console so I am sure the
white-space formatting is wrong.  These two spots should be fixed as
well as they are using the wrong flag type.

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index ec209f4d3312..ee62ec469ab3 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -343,7 +343,7 @@ static struct ip6_tnl *ip6gre_tunnel_locate(struct net *net,
                goto failed_free;

        /* Can use a lockless transmit, unless we generate output sequences */
-       if (!(nt->parms.o_flags & GRE_SEQ))
+       if (!(nt->parms.o_flags & TUNNEL_SEQ))
                dev->features |= NETIF_F_LLTX;

        dev_hold(dev);
@@ -1314,7 +1314,7 @@ static int ip6gre_newlink(struct net *src_net,
struct net_device *dev,
        dev->features           |= GRE6_FEATURES;
        dev->hw_features        |= GRE6_FEATURES;

-       if (!(nt->parms.o_flags & GRE_SEQ)) {
+       if (!(nt->parms.o_flags & TUNNEL_SEQ)) {
                /* TCP segmentation offload is not supported when we
                 * generate output sequences.
                 */

^ permalink raw reply related

* Re: [PATCH net-next] ip6_gre: Fix MTU setting
From: Alexander Duyck @ 2016-05-09 23:10 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <CALx6S35FbaG9jiM4diNmKC+=cSCi+3t7WM1AXxi_c3iigsUu-A@mail.gmail.com>

On Mon, May 9, 2016 at 3:49 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, May 9, 2016 at 3:44 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
>>> configuration. For hard_header_len and mtu calculation include
>>> IPv6 header and encapsulation overhead.
>>>
>>> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
>>> the configuration. Revert to setting hard_header_len instead of
>>> needed_headroom.
>>>
>>> Tested:
>>>
>>> ./ip link add name tun8 type ip6gretap remote \
>>> 2401:db00:20:911a:face:0:27:0 local \
>>> 2401:db00:20:911a:face:0:25:0 ttl 225
>>>
>>> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>>>
>>> ./ip link add name tun8 type ip6gretap remote \
>>> 2401:db00:20:911a:face:0:27:0 local \
>>> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>>>
>>> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>>>
>>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>>
>> So this gives me the correct result now.  However we still need patch
>> 3 / 11 from the set you submitted next week in order to be able to
>> even transmit because the flags are otherwise mangled.
>>
>> Tested-by: Alexander Duyck <aduyck@mirantis.com>
>
> Okay, I'll repost the flags patch as a standalone. Do you have any
> other issues then in this path?

This piece no.

Like I said in the other email I did find two other issues.  First in
__gre6_xmit I think you want to set the inner protocol to "protocol",
not "proto".  Second is the issue with ip6_tnl_xmit stomping on
skb->transport_header.  The second one is the big one that is blocking
things badly right now.  That is the one that keeps me from doing
anything with any sort of offload as the headers are jumping around
out of order with the outer transport header having an offset that is
further in than the inner mac header.

I'd be fine with you submitting the whole set as one piece with the
other 11 patches you had just as long as we get those core pieces
fixed in the patch set.

- Alex

^ permalink raw reply

* Re: [PATCH v3 1/3] net: phy: add phy_ethtool_{get|set}_link_ksettings
From: David Decotigny @ 2016-05-09 23:01 UTC (permalink / raw)
  To: Philippe Reynes
  Cc: fugang.duan, Florian Fainelli, David S. Miller, Ben Hutchings,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1462832383-25541-2-git-send-email-tremyfr@gmail.com>

On Mon, May 9, 2016 at 3:19 PM, Philippe Reynes <tremyfr@gmail.com> wrote:
> Ethtool callbacks {get|set}_link_ksettings are often the same, so
> we add two generics functions phy_ethtool_{get|set}_link_ksettings
> to avoid writing severals times the same function.
>
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
> ---
>  drivers/net/phy/phy.c |   24 ++++++++++++++++++++++++
>  include/linux/phy.h   |    4 ++++
>  2 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 6f221c8..603e8db 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1347,3 +1347,27 @@ void phy_ethtool_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
>                 phydev->drv->get_wol(phydev, wol);
>  }
>  EXPORT_SYMBOL(phy_ethtool_get_wol);
> +
> +int phy_ethtool_get_link_ksettings(struct net_device *ndev,
> +                                  struct ethtool_link_ksettings *cmd)
> +{
> +       struct phy_device *phydev = ndev->phydev;
> +
> +       if (!phydev)
> +               return -ENODEV;
> +
> +       return phy_ethtool_ksettings_get(phydev, cmd);
> +}
> +EXPORT_SYMBOL(phy_ethtool_get_link_ksettings);
> +
> +int phy_ethtool_set_link_ksettings(struct net_device *ndev,
> +                                  const struct ethtool_link_ksettings *cmd)
> +{
> +       struct phy_device *phydev = ndev->phydev;
> +
> +       if (!phydev)
> +               return -ENODEV;
> +
> +       return phy_ethtool_ksettings_set(phydev, cmd);
> +}
> +EXPORT_SYMBOL(phy_ethtool_set_link_ksettings);
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index be3f83b..2d24b28 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -829,6 +829,10 @@ int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data);
>  int phy_ethtool_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol);
>  void phy_ethtool_get_wol(struct phy_device *phydev,
>                          struct ethtool_wolinfo *wol);
> +int phy_ethtool_get_link_ksettings(struct net_device *ndev,
> +                                  struct ethtool_link_ksettings *cmd);
> +int phy_ethtool_set_link_ksettings(struct net_device *ndev,
> +                                  const struct ethtool_link_ksettings *cmd);
>
>  int __init mdio_bus_init(void);
>  void mdio_bus_exit(void);
> --
> 1.7.4.4
>

Acked-By: David Decotigny <decot@googlers.com>

^ permalink raw reply

* Re: [PATCH net-next] ip6_gre: Fix MTU setting
From: Tom Herbert @ 2016-05-09 22:49 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <CAKgT0UdyJyALd6DHA4GeUQtS08MusBTU880A5fNtMbMu_r55Sg@mail.gmail.com>

On Mon, May 9, 2016 at 3:44 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
>> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
>> configuration. For hard_header_len and mtu calculation include
>> IPv6 header and encapsulation overhead.
>>
>> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
>> the configuration. Revert to setting hard_header_len instead of
>> needed_headroom.
>>
>> Tested:
>>
>> ./ip link add name tun8 type ip6gretap remote \
>> 2401:db00:20:911a:face:0:27:0 local \
>> 2401:db00:20:911a:face:0:25:0 ttl 225
>>
>> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>>
>> ./ip link add name tun8 type ip6gretap remote \
>> 2401:db00:20:911a:face:0:27:0 local \
>> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>>
>> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>>
>> Signed-off-by: Tom Herbert <tom@herbertland.com>
>
> So this gives me the correct result now.  However we still need patch
> 3 / 11 from the set you submitted next week in order to be able to
> even transmit because the flags are otherwise mangled.
>
> Tested-by: Alexander Duyck <aduyck@mirantis.com>

Okay, I'll repost the flags patch as a standalone. Do you have any
other issues then in this path?

Tom

^ permalink raw reply

* Re: [PATCH net-next] ip6_gre: Fix MTU setting
From: Alexander Duyck @ 2016-05-09 22:44 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <1462832909-553684-1-git-send-email-tom@herbertland.com>

On Mon, May 9, 2016 at 3:28 PM, Tom Herbert <tom@herbertland.com> wrote:
> In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
> configuration. For hard_header_len and mtu calculation include
> IPv6 header and encapsulation overhead.
>
> In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
> the configuration. Revert to setting hard_header_len instead of
> needed_headroom.
>
> Tested:
>
> ./ip link add name tun8 type ip6gretap remote \
> 2401:db00:20:911a:face:0:27:0 local \
> 2401:db00:20:911a:face:0:25:0 ttl 225
>
> Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.
>
> ./ip link add name tun8 type ip6gretap remote \
> 2401:db00:20:911a:face:0:27:0 local \
> 2401:db00:20:911a:face:0:25:0 ttl 225 okey 123
>
> Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.
>
> Signed-off-by: Tom Herbert <tom@herbertland.com>

So this gives me the correct result now.  However we still need patch
3 / 11 from the set you submitted next week in order to be able to
even transmit because the flags are otherwise mangled.

Tested-by: Alexander Duyck <aduyck@mirantis.com>

^ permalink raw reply

* Re: [PATCH v3 net-next 00/11] ipv6: Enable GUEoIPv6 and more fixes for v6 tunneling
From: Alexander Duyck @ 2016-05-09 22:32 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <CALx6S37htan22_ssFH8J52eaKyt2gc7su-YfDXMC4e-TPfJbcQ@mail.gmail.com>

On Mon, May 9, 2016 at 2:37 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, May 9, 2016 at 2:35 PM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, May 9, 2016 at 10:32 AM, Alexander Duyck
>> <alexander.duyck@gmail.com> wrote:
>>> On Mon, May 9, 2016 at 9:56 AM, Tom Herbert <tom@herbertland.com> wrote:
>>>> On Fri, May 6, 2016 at 8:03 PM, Alexander Duyck
>>>> <alexander.duyck@gmail.com> wrote:
>>>>> On Fri, May 6, 2016 at 7:11 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>>> On Fri, May 6, 2016 at 7:03 PM, Alexander Duyck
>>>>>> <alexander.duyck@gmail.com> wrote:
>>>>>>> On Fri, May 6, 2016 at 6:57 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>>>>> On Fri, May 6, 2016 at 6:09 PM, Alexander Duyck
>>>>>>>> <alexander.duyck@gmail.com> wrote:
>>>>>>>>> On Fri, May 6, 2016 at 3:11 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>>>>>>> This patch set:
>>>>>>>>>>   - Fixes GRE6 to process translate flags correctly from configuration
>>>>>>>>>>   - Adds support for GSO and GRO for ip6ip6 and ip4ip6
>>>>>>>>>>   - Add support for FOU and GUE in IPv6
>>>>>>>>>>   - Support GRE, ip6ip6 and ip4ip6 over FOU/GUE
>>>>>>>>>>   - Fixes ip6_input to deal with UDP encapsulations
>>>>>>>>>>   - Some other minor fixes
>>>>>>>>>>
>>>>>>>>>> v2:
>>>>>>>>>>   - Removed a check of GSO types in MPLS
>>>>>>>>>>   - Define GSO type SKB_GSO_IPXIP6 and SKB_GSO_IPXIP4 (based on input
>>>>>>>>>>     from Alexander)
>>>>>>>>>>   - Don't define GSO types specifally for IP6IP6 and IP4IP6, above
>>>>>>>>>>     fix makes that uncessary
>>>>>>>>>>   - Don't bother clearing encapsulation flag in UDP tunnel segment
>>>>>>>>>>     (another item suggested by Alexander).
>>>>>>>>>>
>>>>>>>>>> v3:
>>>>>>>>>>   - Address some minor comments from Alexander
>>>>>>>>>>
>>>>>>>>>> Tested:
>>>>>>>>>>    Tested a variety of case, but not the full matrix (which is quite
>>>>>>>>>>    large now). Most of the obivous cases (e.g. GRE) work fine. Still
>>>>>>>>>>    some issues probably with GSO/GRO being effective in all cases.
>>>>>>>>>>
>>>>>>>>>>     - IPv4/GRE/GUE/IPv6 with RCO
>>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>>         6616 Mbps
>>>>>>>>>>       200 TCP_RR
>>>>>>>>>>         1244043 tps
>>>>>>>>>>         141/243/446 90/95/99% latencies
>>>>>>>>>>         86.61% CPU utilization
>>>>>>>>>>     - IPv6/GRE/GUE/IPv6 with RCO
>>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>>         6940 Mbps
>>>>>>>>>>       200 TCP_RR
>>>>>>>>>>         1270903 tps
>>>>>>>>>>         138/236/440 90/95/99% latencies
>>>>>>>>>>         87.51% CPU utilization
>>>>>>>>>>
>>>>>>>>>>      - IP6IP6
>>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>>         2576 Mbps
>>>>>>>>>>       200 TCP_RR
>>>>>>>>>>         498981 tps
>>>>>>>>>>         388/498/631 90/95/99% latencies
>>>>>>>>>>         19.75% CPU utilization (1 CPU saturated)
>>>>>>>>>>
>>>>>>>>>>      - IP6IP6/GUE/IPv6 with RCO
>>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>>         1854 Mbps
>>>>>>>>>>       200 TCP_RR
>>>>>>>>>>         1233818 tps
>>>>>>>>>>         143/244/451 90/95/99% latencies
>>>>>>>>>>         87.57 CPU utilization
>>>>>>>>>>
>>>>>>>>>>      - IP4IP6
>>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>>       200 TCP_RR
>>>>>>>>>>         763774 tps
>>>>>>>>>>         250/318/466 90/95/99% latencies
>>>>>>>>>>         35.25% CPU utilization (1 CPU saturated)
>>>>>>>>>>
>>>>>>>>>>      - GRE with keyid
>>>>>>>>>>       200 TCP_RR
>>>>>>>>>>         744173 tps
>>>>>>>>>>         258/332/461 90/95/99% latencies
>>>>>>>>>>         34.59% CPU utilization (1 CPU saturated)
>>>>>>>>>
>>>>>>>>> So I tried testing your patch set and it looks like I cannot get GRE
>>>>>>>>> working for any netperf test.  If I pop the patches off it is even
>>>>>>>>> worse since it looks like patch 3 fixes some tunnel flags issues, but
>>>>>>>>> still doesn't resolve all the issues introduced with b05229f44228
>>>>>>>>> ("gre6: Cleanup GREv6 transmit path, call common GRE functions").
>>>>>>>>> Reverting the entire patch seems to resolve the issues, but I will try
>>>>>>>>> to pick it apart tonight to see if I can find the other issues that
>>>>>>>>> weren't addressed in this patch series.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Can you give details about configuration, test you're running, and HW?
>>>>>>>
>>>>>>> The issue looks like it may be specific to ip6gretap.  I'm running the
>>>>>>> test over an i40e adapter, but it shouldn't make much difference.  I'm
>>>>>>> thinking it may have something to do with the MTU configuration as
>>>>>>> that is one of the things I am noticing has changed between the
>>>>>>> working and the broken version of the code.
>>>>>>>
>>>>>> I'm not seeing any issue with configuring:
>>>>>>
>>>>>> ip link add name tun8 type ip6gretap remote
>>>>>> 2401:db00:20:911a:face:0:27:0 local 2401:db00:20:911a:face:0:25:0 ttl
>>>>>> 225
>>>>>>
>>>>>> MTU issues would not surprise me with IPv6 though. This is part of the
>>>>>> area of code that seems drastically different than what IPv4 is doing.
>>>>>
>>>>> I am also using a key.
>>>>>
>>>>>         ip link add $name type ip6gretap key $net \
>>>>>                 local fec0::1 remote $addr6 ttl 225 dev $PF0
>>>>>
>>>> I don't see any issue with key enabled.
>>>>
>>>>> Does the device you are using support any kind of checksum offload for
>>>>> inner headers on GRE tunnels?  It looks like if I turn off checksums
>>>>
>>>> I don't believe so.
>>>>
>>>>> and correct the MTU I can then send traffic without issues.  I'd say
>>>>> that the Tx cleanup probably introduced 3 regressions.  The first one
>>>>> you addressed in patch 3 which fixes the flags.  The second being the
>>>>> fact that the MTU is wrong, and the third being something that
>>>>> apparently broke checksum and maybe segmentation offload for
>>>>> ip6gretap.
>>>>>
>>>> The MTU can be set in place in IPv6 code that doesn't exist in Ipv4. I
>>>> am especially wondering about the "if (p->flags & IP6_TNL_F_CAP_XMIT)"
>>>> block.
>>
>> So I think I have figured out the MTU problem.  You need to go through
>> and audit the spots where you are using GRE_ flags instead of TUNNEL_
>> flags.  Specifically in ip6gre_tnl_link_config you are using GRE_
>> flags to test o_flags which is configured to use the TUNNEL_ prefixed
>> flags.  When I changed those out then the MTU started coming out
>> correct again.  There were a couple other places you were using
>> GRE_SEQ where you should have been using TUNNEL_SEQ as well so you
>> could probably clean those up and add them to patch 3 of your set that
>> was fixing the flags so that they should be TUNNEL_ prefixed.
>>
>>>>> Really I think the transmit path cleanup should have probably been
>>>>> broken down into a set of patches rather than slamming it in all in
>>>>> one block.  I can spend some time next week trying to sort it out if
>>>>> you don't have any hardware that supports GRE segmentation or checksum
>>>>> offload.  If worse comes to worse I will just try breaking the revert
>>>>> down into a set of smaller patches so I can figure out exactly which
>>>>> change broke things.
>>>>>
>>>> I am still trying to reproduce.
>>>
>>> What NICs are you testing with?  Depending on the NIC I might be able
>>> to point you in the direction of something that can reproduce the
>>> issue.
>>>
>>> At this point I am thinking it is an issue with a header offset since
>>> I believe GSO resets all that and probably corrects the issue.
>>
>> I'm still doing some digging on my end.  I'm hoping to have this
>> figured out by the end of today.
>>
>
> Yes, I will be sending a patch shortly for that if you can try it.

Yes.  I will be available to test whatever you can provide.

In the meantime I have identified two other issues.

1.  skb_set_inner_protocol is being called with the wrong value in
__gre6_xmit.  It should be passed protocol, not proto.

2.  If __gre6_xmit is going to use ip6_tnl_xmit then we cannot clobber
skb->transport_header as we currently do inside of ip6_tnl_xmit
because it creates an invalid value for the offset.  I believe that
would have broken FOU/GUE over the tunnel as well since the transport
header would be needed to support offloads.

If you can get these two issues, plus the one flags issue I mentioned
earlier it should resolve things and make it so that we can go back to
getting ip6gretap working with hardware offloads again.

- Alex

^ permalink raw reply

* [PATCH net-next] ip6_gre: Fix MTU setting
From: Tom Herbert @ 2016-05-09 22:28 UTC (permalink / raw)
  To: davem, netdev, alexander.duyck; +Cc: kernel-team

In ip6gre_tnl_link_config set t->tun_len and t->hlen correctly for the
configuration. For hard_header_len and mtu calculation include
IPv6 header and encapsulation overhead.

In ip6gre_tunnel_init_common set t->tun_len and t->hlen correctly for
the configuration. Revert to setting hard_header_len instead of
needed_headroom.

Tested:

./ip link add name tun8 type ip6gretap remote \
2401:db00:20:911a:face:0:27:0 local \
2401:db00:20:911a:face:0:25:0 ttl 225

Gives MTU of 1434. That is equal to 1500 - 40 - 14 - 4 - 8.

./ip link add name tun8 type ip6gretap remote \
2401:db00:20:911a:face:0:27:0 local \
2401:db00:20:911a:face:0:25:0 ttl 225 okey 123

Gives MTU of 1430. That is equal to 1500 - 40 - 14 - 4 - 8 - 4.

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

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 47b671a..6d0aa94 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -700,7 +700,7 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
 	struct net_device *dev = t->dev;
 	struct __ip6_tnl_parm *p = &t->parms;
 	struct flowi6 *fl6 = &t->fl.u.ip6;
-	int addend = sizeof(struct ipv6hdr) + 4;
+	int t_hlen;
 
 	if (dev->type != ARPHRD_ETHER) {
 		memcpy(dev->dev_addr, &p->laddr, sizeof(struct in6_addr));
@@ -727,16 +727,11 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
 	else
 		dev->flags &= ~IFF_POINTOPOINT;
 
-	/* Precalculate GRE options length */
-	if (t->parms.o_flags&(GRE_CSUM|GRE_KEY|GRE_SEQ)) {
-		if (t->parms.o_flags&GRE_CSUM)
-			addend += 4;
-		if (t->parms.o_flags&GRE_KEY)
-			addend += 4;
-		if (t->parms.o_flags&GRE_SEQ)
-			addend += 4;
-	}
-	t->hlen = addend;
+	t->tun_hlen = gre_calc_hlen(t->parms.o_flags);
+
+	t->hlen = t->tun_hlen;
+
+	t_hlen = t->hlen + sizeof(struct ipv6hdr);
 
 	if (p->flags & IP6_TNL_F_CAP_XMIT) {
 		int strict = (ipv6_addr_type(&p->raddr) &
@@ -750,10 +745,11 @@ static void ip6gre_tnl_link_config(struct ip6_tnl *t, int set_mtu)
 			return;
 
 		if (rt->dst.dev) {
-			dev->hard_header_len = rt->dst.dev->hard_header_len + addend;
+			dev->hard_header_len = rt->dst.dev->hard_header_len +
+					       t_hlen;
 
 			if (set_mtu) {
-				dev->mtu = rt->dst.dev->mtu - addend;
+				dev->mtu = rt->dst.dev->mtu - t_hlen;
 				if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
 					dev->mtu -= 8;
 				if (dev->type == ARPHRD_ETHER)
@@ -1027,11 +1023,12 @@ static int ip6gre_tunnel_init_common(struct net_device *dev)
 
 	tunnel->tun_hlen = gre_calc_hlen(tunnel->parms.o_flags);
 
-	t_hlen = tunnel->hlen + sizeof(struct ipv6hdr);
+	tunnel->hlen = tunnel->tun_hlen;
 
-	dev->needed_headroom	= LL_MAX_HEADER + t_hlen + 4;
-	dev->mtu		= ETH_DATA_LEN - t_hlen - 4;
+	t_hlen = tunnel->hlen + sizeof(struct ipv6hdr);
 
+	dev->hard_header_len = LL_MAX_HEADER + t_hlen;
+	dev->mtu = ETH_DATA_LEN - t_hlen;
 	if (!(tunnel->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT))
 		dev->mtu -= 8;
 
-- 
2.8.0.rc2

^ permalink raw reply related

* [PATCH v3 3/3] net: ethernet: fec: use phy_ethtool_{get|set}_link_ksettings
From: Philippe Reynes @ 2016-05-09 22:19 UTC (permalink / raw)
  To: fugang.duan, f.fainelli, davem, decot, ben
  Cc: netdev, linux-kernel, Philippe Reynes
In-Reply-To: <1462832383-25541-1-git-send-email-tremyfr@gmail.com>

There are two generics functions phy_ethtool_{get|set}_link_ksettings,
so we can use them instead of defining the same code in the driver.

Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
 drivers/net/ethernet/freescale/fec_main.c |   26 ++------------------------
 1 files changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 9d6e35c..ca2cccc 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2061,28 +2061,6 @@ static void fec_enet_mii_remove(struct fec_enet_private *fep)
 	}
 }
 
-static int fec_enet_get_link_ksettings(struct net_device *ndev,
-				       struct ethtool_link_ksettings *cmd)
-{
-	struct phy_device *phydev = ndev->phydev;
-
-	if (!phydev)
-		return -ENODEV;
-
-	return phy_ethtool_ksettings_get(phydev, cmd);
-}
-
-static int fec_enet_set_link_ksettings(struct net_device *ndev,
-				       const struct ethtool_link_ksettings *cmd)
-{
-	struct phy_device *phydev = ndev->phydev;
-
-	if (!phydev)
-		return -ENODEV;
-
-	return phy_ethtool_ksettings_set(phydev, cmd);
-}
-
 static void fec_enet_get_drvinfo(struct net_device *ndev,
 				 struct ethtool_drvinfo *info)
 {
@@ -2581,8 +2559,8 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
 	.set_tunable		= fec_enet_set_tunable,
 	.get_wol		= fec_enet_get_wol,
 	.set_wol		= fec_enet_set_wol,
-	.get_link_ksettings	= fec_enet_get_link_ksettings,
-	.set_link_ksettings	= fec_enet_set_link_ksettings,
+	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
+	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
 };
 
 static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH v3 2/3] net: ethernet: fec: use phydev from struct net_device
From: Philippe Reynes @ 2016-05-09 22:19 UTC (permalink / raw)
  To: fugang.duan, f.fainelli, davem, decot, ben
  Cc: netdev, linux-kernel, Philippe Reynes
In-Reply-To: <1462832383-25541-1-git-send-email-tremyfr@gmail.com>

The private structure contain a pointer to phydev, but the structure
net_device already contain such pointer. So we can remove the pointer
phydev in the private structure, and update the driver to use the one
contained in struct net_device.

Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
 drivers/net/ethernet/freescale/fec.h      |    1 -
 drivers/net/ethernet/freescale/fec_main.c |   49 ++++++++++++----------------
 2 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 195122e..f58f9ea 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -517,7 +517,6 @@ struct fec_enet_private {
 
 	/* Phylib and MDIO interface */
 	struct	mii_bus *mii_bus;
-	struct	phy_device *phy_dev;
 	int	mii_timeout;
 	uint	phy_speed;
 	phy_interface_t	phy_interface;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index c9f77c3..9d6e35c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -967,10 +967,10 @@ fec_restart(struct net_device *ndev)
 			rcntl &= ~(1 << 8);
 
 		/* 1G, 100M or 10M */
-		if (fep->phy_dev) {
-			if (fep->phy_dev->speed == SPEED_1000)
+		if (ndev->phydev) {
+			if (ndev->phydev->speed == SPEED_1000)
 				ecntl |= (1 << 5);
-			else if (fep->phy_dev->speed == SPEED_100)
+			else if (ndev->phydev->speed == SPEED_100)
 				rcntl &= ~(1 << 9);
 			else
 				rcntl |= (1 << 9);
@@ -991,7 +991,7 @@ fec_restart(struct net_device *ndev)
 			 */
 			cfgr = (fep->phy_interface == PHY_INTERFACE_MODE_RMII)
 				? BM_MIIGSK_CFGR_RMII : BM_MIIGSK_CFGR_MII;
-			if (fep->phy_dev && fep->phy_dev->speed == SPEED_10)
+			if (ndev->phydev && ndev->phydev->speed == SPEED_10)
 				cfgr |= BM_MIIGSK_CFGR_FRCONT_10M;
 			writel(cfgr, fep->hwp + FEC_MIIGSK_CFGR);
 
@@ -1005,7 +1005,7 @@ fec_restart(struct net_device *ndev)
 	/* enable pause frame*/
 	if ((fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) ||
 	    ((fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) &&
-	     fep->phy_dev && fep->phy_dev->pause)) {
+	     ndev->phydev && ndev->phydev->pause)) {
 		rcntl |= FEC_ENET_FCE;
 
 		/* set FIFO threshold parameter to reduce overrun */
@@ -1685,7 +1685,7 @@ static void fec_get_mac(struct net_device *ndev)
 static void fec_enet_adjust_link(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
-	struct phy_device *phy_dev = fep->phy_dev;
+	struct phy_device *phy_dev = ndev->phydev;
 	int status_change = 0;
 
 	/* Prevent a state halted on mii error */
@@ -1885,8 +1885,6 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 	int phy_id;
 	int dev_id = fep->dev_id;
 
-	fep->phy_dev = NULL;
-
 	if (fep->phy_node) {
 		phy_dev = of_phy_connect(ndev, fep->phy_node,
 					 &fec_enet_adjust_link, 0,
@@ -1934,7 +1932,6 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 
 	phy_dev->advertising = phy_dev->supported;
 
-	fep->phy_dev = phy_dev;
 	fep->link = 0;
 	fep->full_duplex = 0;
 
@@ -2067,8 +2064,7 @@ static void fec_enet_mii_remove(struct fec_enet_private *fep)
 static int fec_enet_get_link_ksettings(struct net_device *ndev,
 				       struct ethtool_link_ksettings *cmd)
 {
-	struct fec_enet_private *fep = netdev_priv(ndev);
-	struct phy_device *phydev = fep->phy_dev;
+	struct phy_device *phydev = ndev->phydev;
 
 	if (!phydev)
 		return -ENODEV;
@@ -2079,8 +2075,7 @@ static int fec_enet_get_link_ksettings(struct net_device *ndev,
 static int fec_enet_set_link_ksettings(struct net_device *ndev,
 				       const struct ethtool_link_ksettings *cmd)
 {
-	struct fec_enet_private *fep = netdev_priv(ndev);
-	struct phy_device *phydev = fep->phy_dev;
+	struct phy_device *phydev = ndev->phydev;
 
 	if (!phydev)
 		return -ENODEV;
@@ -2220,7 +2215,7 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
-	if (!fep->phy_dev)
+	if (!ndev->phydev)
 		return -ENODEV;
 
 	if (pause->tx_pause != pause->rx_pause) {
@@ -2236,17 +2231,17 @@ static int fec_enet_set_pauseparam(struct net_device *ndev,
 	fep->pause_flag |= pause->autoneg ? FEC_PAUSE_FLAG_AUTONEG : 0;
 
 	if (pause->rx_pause || pause->autoneg) {
-		fep->phy_dev->supported |= ADVERTISED_Pause;
-		fep->phy_dev->advertising |= ADVERTISED_Pause;
+		ndev->phydev->supported |= ADVERTISED_Pause;
+		ndev->phydev->advertising |= ADVERTISED_Pause;
 	} else {
-		fep->phy_dev->supported &= ~ADVERTISED_Pause;
-		fep->phy_dev->advertising &= ~ADVERTISED_Pause;
+		ndev->phydev->supported &= ~ADVERTISED_Pause;
+		ndev->phydev->advertising &= ~ADVERTISED_Pause;
 	}
 
 	if (pause->autoneg) {
 		if (netif_running(ndev))
 			fec_stop(ndev);
-		phy_start_aneg(fep->phy_dev);
+		phy_start_aneg(ndev->phydev);
 	}
 	if (netif_running(ndev)) {
 		napi_disable(&fep->napi);
@@ -2362,8 +2357,7 @@ static int fec_enet_get_sset_count(struct net_device *dev, int sset)
 
 static int fec_enet_nway_reset(struct net_device *dev)
 {
-	struct fec_enet_private *fep = netdev_priv(dev);
-	struct phy_device *phydev = fep->phy_dev;
+	struct phy_device *phydev = dev->phydev;
 
 	if (!phydev)
 		return -ENODEV;
@@ -2594,7 +2588,7 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
 static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
-	struct phy_device *phydev = fep->phy_dev;
+	struct phy_device *phydev = ndev->phydev;
 
 	if (!netif_running(ndev))
 		return -EINVAL;
@@ -2849,7 +2843,7 @@ fec_enet_open(struct net_device *ndev)
 		goto err_enet_mii_probe;
 
 	napi_enable(&fep->napi);
-	phy_start(fep->phy_dev);
+	phy_start(ndev->phydev);
 	netif_tx_start_all_queues(ndev);
 
 	device_set_wakeup_enable(&ndev->dev, fep->wol_flag &
@@ -2873,7 +2867,7 @@ fec_enet_close(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
-	phy_stop(fep->phy_dev);
+	phy_stop(ndev->phydev);
 
 	if (netif_device_present(ndev)) {
 		napi_disable(&fep->napi);
@@ -2881,8 +2875,7 @@ fec_enet_close(struct net_device *ndev)
 		fec_stop(ndev);
 	}
 
-	phy_disconnect(fep->phy_dev);
-	fep->phy_dev = NULL;
+	phy_disconnect(ndev->phydev);
 
 	fec_enet_clk_enable(ndev, false);
 	pinctrl_pm_select_sleep_state(&fep->pdev->dev);
@@ -3510,7 +3503,7 @@ static int __maybe_unused fec_suspend(struct device *dev)
 	if (netif_running(ndev)) {
 		if (fep->wol_flag & FEC_WOL_FLAG_ENABLE)
 			fep->wol_flag |= FEC_WOL_FLAG_SLEEP_ON;
-		phy_stop(fep->phy_dev);
+		phy_stop(ndev->phydev);
 		napi_disable(&fep->napi);
 		netif_tx_lock_bh(ndev);
 		netif_device_detach(ndev);
@@ -3570,7 +3563,7 @@ static int __maybe_unused fec_resume(struct device *dev)
 		netif_device_attach(ndev);
 		netif_tx_unlock_bh(ndev);
 		napi_enable(&fep->napi);
-		phy_start(fep->phy_dev);
+		phy_start(ndev->phydev);
 	}
 	rtnl_unlock();
 
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH v3 1/3] net: phy: add phy_ethtool_{get|set}_link_ksettings
From: Philippe Reynes @ 2016-05-09 22:19 UTC (permalink / raw)
  To: fugang.duan, f.fainelli, davem, decot, ben
  Cc: netdev, linux-kernel, Philippe Reynes
In-Reply-To: <1462832383-25541-1-git-send-email-tremyfr@gmail.com>

Ethtool callbacks {get|set}_link_ksettings are often the same, so
we add two generics functions phy_ethtool_{get|set}_link_ksettings
to avoid writing severals times the same function.

Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
 drivers/net/phy/phy.c |   24 ++++++++++++++++++++++++
 include/linux/phy.h   |    4 ++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 6f221c8..603e8db 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1347,3 +1347,27 @@ void phy_ethtool_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
 		phydev->drv->get_wol(phydev, wol);
 }
 EXPORT_SYMBOL(phy_ethtool_get_wol);
+
+int phy_ethtool_get_link_ksettings(struct net_device *ndev,
+				   struct ethtool_link_ksettings *cmd)
+{
+	struct phy_device *phydev = ndev->phydev;
+
+	if (!phydev)
+		return -ENODEV;
+
+	return phy_ethtool_ksettings_get(phydev, cmd);
+}
+EXPORT_SYMBOL(phy_ethtool_get_link_ksettings);
+
+int phy_ethtool_set_link_ksettings(struct net_device *ndev,
+				   const struct ethtool_link_ksettings *cmd)
+{
+	struct phy_device *phydev = ndev->phydev;
+
+	if (!phydev)
+		return -ENODEV;
+
+	return phy_ethtool_ksettings_set(phydev, cmd);
+}
+EXPORT_SYMBOL(phy_ethtool_set_link_ksettings);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index be3f83b..2d24b28 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -829,6 +829,10 @@ int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data);
 int phy_ethtool_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol);
 void phy_ethtool_get_wol(struct phy_device *phydev,
 			 struct ethtool_wolinfo *wol);
+int phy_ethtool_get_link_ksettings(struct net_device *ndev,
+				   struct ethtool_link_ksettings *cmd);
+int phy_ethtool_set_link_ksettings(struct net_device *ndev,
+				   const struct ethtool_link_ksettings *cmd);
 
 int __init mdio_bus_init(void);
 void mdio_bus_exit(void);
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH v3 0/3] net: phy: add phy_ethtool_{get|set}_link_ksettings
From: Philippe Reynes @ 2016-05-09 22:19 UTC (permalink / raw)
  To: fugang.duan, f.fainelli, davem, decot, ben
  Cc: netdev, linux-kernel, Philippe Reynes

Ethtool callbacks {get|set}_link_ksettings may be the
same for many drivers. So we add two generics callbacks
phy_ethtool_{get|set}_link_ksettings.

To use those generics callbacks, the ethernet driver must
use the pointer phydev contained in struct net_device, and
not use a private structure to store this pointer.

Changelog:
v3:
- rename function to phy_ethtool_{get|set}_link_ksettings
- move code to net/phy/phy.c
  This feedback were provided by David Decotigny
v2:
- use generic function instead of macro
- ethernet driver use the pointer phydev provided by struct net_device
  Those idea were provided by Ben Hutchings,
  and Florian Fainelli acknowledge them.

Philippe Reynes (3):
  net: phy: add phy_ethtool_{get|set}_link_ksettings
  net: ethernet: fec: use phydev from struct net_device
  net: ethernet: fec: use phy_ethtool_{get|set}_link_ksettings

 drivers/net/ethernet/freescale/fec.h      |    1 -
 drivers/net/ethernet/freescale/fec_main.c |   71 +++++++++--------------------
 drivers/net/phy/phy.c                     |   24 ++++++++++
 include/linux/phy.h                       |    4 ++
 4 files changed, 49 insertions(+), 51 deletions(-)

-- 
1.7.4.4

^ permalink raw reply

* Re: [PATCH v2] uapi glibc compat: fix compile errors when glibc net/if.h included before linux/if.h
From: Mikko Rapeli @ 2016-05-09 22:15 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Josh Boyer, David S. Miller, Zhang Shengju, Stephen Hemminger,
	Linux-Kernel@Vger. Kernel. Org, Jan Engelhardt, Waldemar Brodkorb,
	Gabriel Laskar, netfilter-devel, libc-alpha, netdev
In-Reply-To: <20160509213535.GA23623@salvia>

On Mon, May 09, 2016 at 11:35:35PM +0200, Pablo Neira Ayuso wrote:
> It seems linux-netdev was not Cc'ed. I cannot find this in David's
> patchwork [1].
> 
> @Mikko: Could you resubmit including netdev@vger.kernel.org? Thanks.

Done: http://patchwork.ozlabs.org/patch/620262/

-Mikko

^ permalink raw reply

* [PATCH v2] uapi glibc compat: fix compile errors when glibc net/if.h included before linux/if.h
From: Mikko Rapeli @ 2016-04-24 15:45 UTC (permalink / raw)
  To: Mikko Rapeli, David S. Miller, Zhang Shengju, Stephen Hemminger,
	linux-kernel
  Cc: Jan Engelhardt, Josh Boyer, Waldemar Brodkorb, Gabriel Laskar,
	netfilter-devel, libc-alpha

glibc's net/if.h contains copies of definitions from linux/if.h and these
conflict and cause build failures if both files are included by application
source code. Changes in uapi headers, which fixed header file dependencies to
include linux/if.h when it was needed, e.g. commit 1ffad83d, made the
net/if.h and linux/if.h incompatibilities visible as build failures for
userspace applications like iproute2 and xtables-addons.

This patch fixes compile errors when glibc net/if.h is included before
linux/if.h:

./linux/if.h:99:21: error: redeclaration of enumerator ‘IFF_NOARP’
./linux/if.h:98:23: error: redeclaration of enumerator ‘IFF_RUNNING’
./linux/if.h:97:26: error: redeclaration of enumerator ‘IFF_NOTRAILERS’
./linux/if.h:96:27: error: redeclaration of enumerator ‘IFF_POINTOPOINT’
./linux/if.h:95:24: error: redeclaration of enumerator ‘IFF_LOOPBACK’
./linux/if.h:94:21: error: redeclaration of enumerator ‘IFF_DEBUG’
./linux/if.h:93:25: error: redeclaration of enumerator ‘IFF_BROADCAST’
./linux/if.h:92:19: error: redeclaration of enumerator ‘IFF_UP’
./linux/if.h:252:8: error: redefinition of ‘struct ifconf’
./linux/if.h:203:8: error: redefinition of ‘struct ifreq’
./linux/if.h:169:8: error: redefinition of ‘struct ifmap’
./linux/if.h:107:23: error: redeclaration of enumerator ‘IFF_DYNAMIC’
./linux/if.h:106:25: error: redeclaration of enumerator ‘IFF_AUTOMEDIA’
./linux/if.h:105:23: error: redeclaration of enumerator ‘IFF_PORTSEL’
./linux/if.h:104:25: error: redeclaration of enumerator ‘IFF_MULTICAST’
./linux/if.h:103:21: error: redeclaration of enumerator ‘IFF_SLAVE’
./linux/if.h:102:22: error: redeclaration of enumerator ‘IFF_MASTER’
./linux/if.h:101:24: error: redeclaration of enumerator ‘IFF_ALLMULTI’
./linux/if.h:100:23: error: redeclaration of enumerator ‘IFF_PROMISC’

The cases where linux/if.h is included before net/if.h need a similar fix in
the glibc side, or the order of include files can be changed userspace
code as a workaround.

This change was tested in x86 userspace on Debian unstable with
scripts/headers_compile_test.sh:

$ make headers_install && \
  cd usr/include && ../../scripts/headers_compile_test.sh -l -k
...
cc -Wall -c -nostdinc -I /usr/lib/gcc/i586-linux-gnu/5/include -I /usr/lib/gcc/i586-linux-gnu/5/include-fixed -I . -I /home/mcfrisk/src/linux-2.6/usr/headers_compile_test_include.2uX2zH -I /home/mcfrisk/src/linux-2.6/usr/headers_compile_test_include.2uX2zH/i586-linux-gnu -o /dev/null ./linux/if.h_libc_before_kernel.h
PASSED libc before kernel test: ./linux/if.h

Reported-by: Jan Engelhardt <jengelh@inai.de>
Reported-by: Josh Boyer <jwboyer@fedoraproject.org>
Reported-by: Stephen Hemminger <shemming@brocade.com>
Reported-by: Waldemar Brodkorb <mail@waldemar-brodkorb.de>
Cc: Gabriel Laskar <gabriel@lse.epita.fr>
Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
---
 include/uapi/linux/if.h          | 28 +++++++++++++++++++++++++
 include/uapi/linux/libc-compat.h | 44 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

v1:
http://marc.info/?l=linux-kernel&m=145485386721798&w=2

v2:
Added handling for net_device_flags IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO
which are unknown to glibc as suggested by David Miller in
http://marc.info/?l=linux-kernel&m=145650410726228&w=2

diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index f802775..e601c8c 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -19,14 +19,20 @@
 #ifndef _LINUX_IF_H
 #define _LINUX_IF_H
 
+#include <linux/libc-compat.h>          /* for compatibility with glibc */
 #include <linux/types.h>		/* for "__kernel_caddr_t" et al	*/
 #include <linux/socket.h>		/* for "struct sockaddr" et al	*/
 #include <linux/compiler.h>		/* for "__user" et al           */
 
+#if __UAPI_DEF_IF_IFNAMSIZ
 #define	IFNAMSIZ	16
+#endif /* __UAPI_DEF_IF_IFNAMSIZ */
 #define	IFALIASZ	256
 #include <linux/hdlc/ioctl.h>
 
+/* For glibc compatibility. An empty enum does not compile. */
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO != 0 && \
+    __UAPI_DEF_IF_NET_DEVICE_FLAGS != 0
 /**
  * enum net_device_flags - &struct net_device flags
  *
@@ -68,6 +74,8 @@
  * @IFF_ECHO: echo sent packets. Volatile.
  */
 enum net_device_flags {
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS
 	IFF_UP				= 1<<0,  /* sysfs */
 	IFF_BROADCAST			= 1<<1,  /* volatile */
 	IFF_DEBUG			= 1<<2,  /* sysfs */
@@ -84,11 +92,17 @@ enum net_device_flags {
 	IFF_PORTSEL			= 1<<13, /* sysfs */
 	IFF_AUTOMEDIA			= 1<<14, /* sysfs */
 	IFF_DYNAMIC			= 1<<15, /* sysfs */
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS */
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO
 	IFF_LOWER_UP			= 1<<16, /* volatile */
 	IFF_DORMANT			= 1<<17, /* volatile */
 	IFF_ECHO			= 1<<18, /* volatile */
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO */
 };
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO != 0 && __UAPI_DEF_IF_NET_DEVICE_FLAGS != 0 */
 
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS
 #define IFF_UP				IFF_UP
 #define IFF_BROADCAST			IFF_BROADCAST
 #define IFF_DEBUG			IFF_DEBUG
@@ -105,9 +119,13 @@ enum net_device_flags {
 #define IFF_PORTSEL			IFF_PORTSEL
 #define IFF_AUTOMEDIA			IFF_AUTOMEDIA
 #define IFF_DYNAMIC			IFF_DYNAMIC
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS */
+
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO
 #define IFF_LOWER_UP			IFF_LOWER_UP
 #define IFF_DORMANT			IFF_DORMANT
 #define IFF_ECHO			IFF_ECHO
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO */
 
 #define IFF_VOLATILE	(IFF_LOOPBACK|IFF_POINTOPOINT|IFF_BROADCAST|IFF_ECHO|\
 		IFF_MASTER|IFF_SLAVE|IFF_RUNNING|IFF_LOWER_UP|IFF_DORMANT)
@@ -166,6 +184,8 @@ enum {
  *	being very small might be worth keeping for clean configuration.
  */
 
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_IFMAP
 struct ifmap {
 	unsigned long mem_start;
 	unsigned long mem_end;
@@ -175,6 +195,7 @@ struct ifmap {
 	unsigned char port;
 	/* 3 bytes spare */
 };
+#endif /* __UAPI_DEF_IF_IFMAP */
 
 struct if_settings {
 	unsigned int type;	/* Type of physical device or protocol */
@@ -200,6 +221,8 @@ struct if_settings {
  * remainder may be interface specific.
  */
 
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_IFREQ
 struct ifreq {
 #define IFHWADDRLEN	6
 	union
@@ -223,6 +246,7 @@ struct ifreq {
 		struct	if_settings ifru_settings;
 	} ifr_ifru;
 };
+#endif /* __UAPI_DEF_IF_IFREQ */
 
 #define ifr_name	ifr_ifrn.ifrn_name	/* interface name 	*/
 #define ifr_hwaddr	ifr_ifru.ifru_hwaddr	/* MAC address 		*/
@@ -249,6 +273,8 @@ struct ifreq {
  * must know all networks accessible).
  */
 
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_IFCONF
 struct ifconf  {
 	int	ifc_len;			/* size of buffer	*/
 	union {
@@ -256,6 +282,8 @@ struct ifconf  {
 		struct ifreq __user *ifcu_req;
 	} ifc_ifcu;
 };
+#endif /* __UAPI_DEF_IF_IFCONF */
+
 #define	ifc_buf	ifc_ifcu.ifcu_buf		/* buffer address	*/
 #define	ifc_req	ifc_ifcu.ifcu_req		/* array of structures	*/
 
diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 7d024ce..d5e38c7 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -51,6 +51,40 @@
 /* We have included glibc headers... */
 #if defined(__GLIBC__)
 
+/* Coordinate with glibc net/if.h header. */
+#if defined(_NET_IF_H)
+
+/* GLIBC headers included first so don't define anything
+ * that would already be defined. */
+
+#define __UAPI_DEF_IF_IFCONF 0
+#define __UAPI_DEF_IF_IFMAP 0
+#define __UAPI_DEF_IF_IFNAMSIZ 0
+#define __UAPI_DEF_IF_IFREQ 0
+/* Everything up to IFF_DYNAMIC, matches net/if.h until glibc 2.23 */
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS 0
+/* For the future if glibc adds IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO */
+#ifndef __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO 1
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO */
+
+#else /* _NET_IF_H */
+
+/* Linux headers included first, and we must define everything
+ * we need. The expectation is that glibc will check the
+ * __UAPI_DEF_* defines and adjust appropriately. */
+
+#define __UAPI_DEF_IF_IFCONF 1
+#define __UAPI_DEF_IF_IFMAP 1
+#define __UAPI_DEF_IF_IFNAMSIZ 1
+#define __UAPI_DEF_IF_IFREQ 1
+/* Everything up to IFF_DYNAMIC, matches net/if.h until glibc 2.23 */
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS 1
+/* For the future if glibc adds IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO */
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO 1
+
+#endif /* _NET_IF_H */
+
 /* Coordinate with glibc netinet/in.h header. */
 #if defined(_NETINET_IN_H)
 
@@ -117,6 +151,16 @@
  * that we need. */
 #else /* !defined(__GLIBC__) */
 
+/* Definitions for if.h */
+#define __UAPI_DEF_IF_IFCONF 1
+#define __UAPI_DEF_IF_IFMAP 1
+#define __UAPI_DEF_IF_IFNAMSIZ 1
+#define __UAPI_DEF_IF_IFREQ 1
+/* Everything up to IFF_DYNAMIC, matches net/if.h until glibc 2.23 */
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS 1
+/* For the future if glibc adds IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO */
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO 1
+
 /* Definitions for in.h */
 #define __UAPI_DEF_IN_ADDR		1
 #define __UAPI_DEF_IN_IPPROTO		1
-- 
2.8.0.rc3

^ permalink raw reply related

* [PATCH v2 net-next] tcp: replace cnt & rtt with struct in pkts_acked()
From: Lawrence Brakmo @ 2016-05-09 21:46 UTC (permalink / raw)
  To: netdev; +Cc: Kernel Team, Neal Cardwell, Eric Dumazet, Yuchung Cheng

Replace 2 arguments (cnt and rtt) in the congestion control modules'
pkts_acked() function with a struct. This will allow adding more
information without having to modify existing congestion control
modules (tcp_nv in particular needs bytes in flight when packet
was sent).

As proposed by Neal Cardwell in his comments to the tcp_nv patch.

Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
 include/net/tcp.h       |  7 ++++++-
 net/ipv4/tcp_bic.c      |  6 +++---
 net/ipv4/tcp_cdg.c      | 14 +++++++-------
 net/ipv4/tcp_cubic.c    |  6 +++---
 net/ipv4/tcp_htcp.c     | 10 +++++-----
 net/ipv4/tcp_illinois.c | 20 ++++++++++----------
 net/ipv4/tcp_input.c    |  8 ++++++--
 net/ipv4/tcp_lp.c       |  6 +++---
 net/ipv4/tcp_vegas.c    |  6 +++---
 net/ipv4/tcp_vegas.h    |  2 +-
 net/ipv4/tcp_veno.c     |  7 ++++---
 net/ipv4/tcp_westwood.c |  7 ++++---
 net/ipv4/tcp_yeah.c     |  7 ++++---
 13 files changed, 59 insertions(+), 47 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 24ec804..dc588c3 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -849,6 +849,11 @@ enum tcp_ca_ack_event_flags {
 
 union tcp_cc_info;
 
+struct ack_sample {
+	u32 pkts_acked;
+	s32 rtt_us;
+};
+
 struct tcp_congestion_ops {
 	struct list_head	list;
 	u32 key;
@@ -872,7 +877,7 @@ struct tcp_congestion_ops {
 	/* new value of cwnd after loss (optional) */
 	u32  (*undo_cwnd)(struct sock *sk);
 	/* hook for packet ack accounting (optional) */
-	void (*pkts_acked)(struct sock *sk, u32 num_acked, s32 rtt_us);
+	void (*pkts_acked)(struct sock *sk, const struct ack_sample *sample);
 	/* get info for inet_diag (optional) */
 	size_t (*get_info)(struct sock *sk, u32 ext, int *attr,
 			   union tcp_cc_info *info);
diff --git a/net/ipv4/tcp_bic.c b/net/ipv4/tcp_bic.c
index fd1405d..f469f1b 100644
--- a/net/ipv4/tcp_bic.c
+++ b/net/ipv4/tcp_bic.c
@@ -197,15 +197,15 @@ static void bictcp_state(struct sock *sk, u8 new_state)
 /* Track delayed acknowledgment ratio using sliding window
  * ratio = (15*ratio + sample) / 16
  */
-static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt)
+static void bictcp_acked(struct sock *sk, const struct ack_sample *sample)
 {
 	const struct inet_connection_sock *icsk = inet_csk(sk);
 
 	if (icsk->icsk_ca_state == TCP_CA_Open) {
 		struct bictcp *ca = inet_csk_ca(sk);
 
-		cnt -= ca->delayed_ack >> ACK_RATIO_SHIFT;
-		ca->delayed_ack += cnt;
+		ca->delayed_ack += sample->pkts_acked - 
+			(ca->delayed_ack >> ACK_RATIO_SHIFT);
 	}
 }
 
diff --git a/net/ipv4/tcp_cdg.c b/net/ipv4/tcp_cdg.c
index ccce8a5..03725b2 100644
--- a/net/ipv4/tcp_cdg.c
+++ b/net/ipv4/tcp_cdg.c
@@ -294,12 +294,12 @@ static void tcp_cdg_cong_avoid(struct sock *sk, u32 ack, u32 acked)
 	ca->shadow_wnd = max(ca->shadow_wnd, ca->shadow_wnd + incr);
 }
 
-static void tcp_cdg_acked(struct sock *sk, u32 num_acked, s32 rtt_us)
+static void tcp_cdg_acked(struct sock *sk, const struct ack_sample *sample)
 {
 	struct cdg *ca = inet_csk_ca(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	if (rtt_us <= 0)
+	if (sample->rtt_us <= 0)
 		return;
 
 	/* A heuristic for filtering delayed ACKs, adapted from:
@@ -307,20 +307,20 @@ static void tcp_cdg_acked(struct sock *sk, u32 num_acked, s32 rtt_us)
 	 * delay and rate based TCP mechanisms." TR 100219A. CAIA, 2010.
 	 */
 	if (tp->sacked_out == 0) {
-		if (num_acked == 1 && ca->delack) {
+		if (sample->pkts_acked == 1 && ca->delack) {
 			/* A delayed ACK is only used for the minimum if it is
 			 * provenly lower than an existing non-zero minimum.
 			 */
-			ca->rtt.min = min(ca->rtt.min, rtt_us);
+			ca->rtt.min = min(ca->rtt.min, sample->rtt_us);
 			ca->delack--;
 			return;
-		} else if (num_acked > 1 && ca->delack < 5) {
+		} else if (sample->pkts_acked > 1 && ca->delack < 5) {
 			ca->delack++;
 		}
 	}
 
-	ca->rtt.min = min_not_zero(ca->rtt.min, rtt_us);
-	ca->rtt.max = max(ca->rtt.max, rtt_us);
+	ca->rtt.min = min_not_zero(ca->rtt.min, sample->rtt_us);
+	ca->rtt.max = max(ca->rtt.max, sample->rtt_us);
 }
 
 static u32 tcp_cdg_ssthresh(struct sock *sk)
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 0ce946e..c99230e 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -437,21 +437,21 @@ static void hystart_update(struct sock *sk, u32 delay)
 /* Track delayed acknowledgment ratio using sliding window
  * ratio = (15*ratio + sample) / 16
  */
-static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt_us)
+static void bictcp_acked(struct sock *sk, const struct ack_sample *sample)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
 	struct bictcp *ca = inet_csk_ca(sk);
 	u32 delay;
 
 	/* Some calls are for duplicates without timetamps */
-	if (rtt_us < 0)
+	if (sample->rtt_us < 0)
 		return;
 
 	/* Discard delay samples right after fast recovery */
 	if (ca->epoch_start && (s32)(tcp_time_stamp - ca->epoch_start) < HZ)
 		return;
 
-	delay = (rtt_us << 3) / USEC_PER_MSEC;
+	delay = (sample->rtt_us << 3) / USEC_PER_MSEC;
 	if (delay == 0)
 		delay = 1;
 
diff --git a/net/ipv4/tcp_htcp.c b/net/ipv4/tcp_htcp.c
index 82f0d9e..4a4d8e7 100644
--- a/net/ipv4/tcp_htcp.c
+++ b/net/ipv4/tcp_htcp.c
@@ -99,7 +99,7 @@ static inline void measure_rtt(struct sock *sk, u32 srtt)
 }
 
 static void measure_achieved_throughput(struct sock *sk,
-					u32 pkts_acked, s32 rtt)
+					const struct ack_sample *sample)
 {
 	const struct inet_connection_sock *icsk = inet_csk(sk);
 	const struct tcp_sock *tp = tcp_sk(sk);
@@ -107,10 +107,10 @@ static void measure_achieved_throughput(struct sock *sk,
 	u32 now = tcp_time_stamp;
 
 	if (icsk->icsk_ca_state == TCP_CA_Open)
-		ca->pkts_acked = pkts_acked;
+		ca->pkts_acked = sample->pkts_acked;
 
-	if (rtt > 0)
-		measure_rtt(sk, usecs_to_jiffies(rtt));
+	if (sample->rtt_us > 0)
+		measure_rtt(sk, usecs_to_jiffies(sample->rtt_us));
 
 	if (!use_bandwidth_switch)
 		return;
@@ -122,7 +122,7 @@ static void measure_achieved_throughput(struct sock *sk,
 		return;
 	}
 
-	ca->packetcount += pkts_acked;
+	ca->packetcount += sample->pkts_acked;
 
 	if (ca->packetcount >= tp->snd_cwnd - (ca->alpha >> 7 ? : 1) &&
 	    now - ca->lasttime >= ca->minRTT &&
diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c
index 2ab9bbb..77ad119 100644
--- a/net/ipv4/tcp_illinois.c
+++ b/net/ipv4/tcp_illinois.c
@@ -82,30 +82,30 @@ static void tcp_illinois_init(struct sock *sk)
 }
 
 /* Measure RTT for each ack. */
-static void tcp_illinois_acked(struct sock *sk, u32 pkts_acked, s32 rtt)
+static void tcp_illinois_acked(struct sock *sk, const struct ack_sample *sample)
 {
 	struct illinois *ca = inet_csk_ca(sk);
 
-	ca->acked = pkts_acked;
+	ca->acked = sample->pkts_acked;
 
 	/* dup ack, no rtt sample */
-	if (rtt < 0)
+	if (sample->rtt_us < 0)
 		return;
 
 	/* ignore bogus values, this prevents wraparound in alpha math */
-	if (rtt > RTT_MAX)
-		rtt = RTT_MAX;
+	if (sample->rtt_us > RTT_MAX)
+		sample->rtt_us = RTT_MAX;
 
 	/* keep track of minimum RTT seen so far */
-	if (ca->base_rtt > rtt)
-		ca->base_rtt = rtt;
+	if (ca->base_rtt > sample->rtt_us)
+		ca->base_rtt = sample->rtt_us;
 
 	/* and max */
-	if (ca->max_rtt < rtt)
-		ca->max_rtt = rtt;
+	if (ca->max_rtt < sample->rtt_us)
+		ca->max_rtt = sample->rtt_us;
 
 	++ca->cnt_rtt;
-	ca->sum_rtt += rtt;
+	ca->sum_rtt += sample->rtt_us;
 }
 
 /* Maximum queuing delay */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a914e06..22a57a8 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3248,8 +3248,12 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
 		tcp_rearm_rto(sk);
 	}
 
-	if (icsk->icsk_ca_ops->pkts_acked)
-		icsk->icsk_ca_ops->pkts_acked(sk, pkts_acked, ca_rtt_us);
+	if (icsk->icsk_ca_ops->pkts_acked) {
+		struct ack_sample sample = {.pkts_acked = pkts_acked,
+					    .rtt_us = ca_rtt_us};
+
+		icsk->icsk_ca_ops->pkts_acked(sk, &sample);
+	}
 
 #if FASTRETRANS_DEBUG > 0
 	WARN_ON((int)tp->sacked_out < 0);
diff --git a/net/ipv4/tcp_lp.c b/net/ipv4/tcp_lp.c
index 1e70fa8..c67ece1 100644
--- a/net/ipv4/tcp_lp.c
+++ b/net/ipv4/tcp_lp.c
@@ -260,13 +260,13 @@ static void tcp_lp_rtt_sample(struct sock *sk, u32 rtt)
  * newReno in increase case.
  * We work it out by following the idea from TCP-LP's paper directly
  */
-static void tcp_lp_pkts_acked(struct sock *sk, u32 num_acked, s32 rtt_us)
+static void tcp_lp_pkts_acked(struct sock *sk, const struct ack_sample *sample)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct lp *lp = inet_csk_ca(sk);
 
-	if (rtt_us > 0)
-		tcp_lp_rtt_sample(sk, rtt_us);
+	if (sample->rtt_us > 0)
+		tcp_lp_rtt_sample(sk, sample->rtt_us);
 
 	/* calc inference */
 	if (tcp_time_stamp > tp->rx_opt.rcv_tsecr)
diff --git a/net/ipv4/tcp_vegas.c b/net/ipv4/tcp_vegas.c
index 13951c4..4c4bac1 100644
--- a/net/ipv4/tcp_vegas.c
+++ b/net/ipv4/tcp_vegas.c
@@ -107,16 +107,16 @@ EXPORT_SYMBOL_GPL(tcp_vegas_init);
  *   o min-filter RTT samples from a much longer window (forever for now)
  *     to find the propagation delay (baseRTT)
  */
-void tcp_vegas_pkts_acked(struct sock *sk, u32 cnt, s32 rtt_us)
+void tcp_vegas_pkts_acked(struct sock *sk, const struct ack_sample *sample)
 {
 	struct vegas *vegas = inet_csk_ca(sk);
 	u32 vrtt;
 
-	if (rtt_us < 0)
+	if (sample->rtt_us < 0)
 		return;
 
 	/* Never allow zero rtt or baseRTT */
-	vrtt = rtt_us + 1;
+	vrtt = sample->rtt_us + 1;
 
 	/* Filter to find propagation delay: */
 	if (vrtt < vegas->baseRTT)
diff --git a/net/ipv4/tcp_vegas.h b/net/ipv4/tcp_vegas.h
index ef9da53..248cfc0 100644
--- a/net/ipv4/tcp_vegas.h
+++ b/net/ipv4/tcp_vegas.h
@@ -17,7 +17,7 @@ struct vegas {
 
 void tcp_vegas_init(struct sock *sk);
 void tcp_vegas_state(struct sock *sk, u8 ca_state);
-void tcp_vegas_pkts_acked(struct sock *sk, u32 cnt, s32 rtt_us);
+void tcp_vegas_pkts_acked(struct sock *sk, const struct ack_sample *sample);
 void tcp_vegas_cwnd_event(struct sock *sk, enum tcp_ca_event event);
 size_t tcp_vegas_get_info(struct sock *sk, u32 ext, int *attr,
 			  union tcp_cc_info *info);
diff --git a/net/ipv4/tcp_veno.c b/net/ipv4/tcp_veno.c
index 0d094b9..40171e1 100644
--- a/net/ipv4/tcp_veno.c
+++ b/net/ipv4/tcp_veno.c
@@ -69,16 +69,17 @@ static void tcp_veno_init(struct sock *sk)
 }
 
 /* Do rtt sampling needed for Veno. */
-static void tcp_veno_pkts_acked(struct sock *sk, u32 cnt, s32 rtt_us)
+static void tcp_veno_pkts_acked(struct sock *sk,
+				const struct ack_sample *sample)
 {
 	struct veno *veno = inet_csk_ca(sk);
 	u32 vrtt;
 
-	if (rtt_us < 0)
+	if (sample->rtt_us < 0)
 		return;
 
 	/* Never allow zero rtt or baseRTT */
-	vrtt = rtt_us + 1;
+	vrtt = sample->rtt_us + 1;
 
 	/* Filter to find propagation delay: */
 	if (vrtt < veno->basertt)
diff --git a/net/ipv4/tcp_westwood.c b/net/ipv4/tcp_westwood.c
index c10732e..4b03a2e 100644
--- a/net/ipv4/tcp_westwood.c
+++ b/net/ipv4/tcp_westwood.c
@@ -99,12 +99,13 @@ static void westwood_filter(struct westwood *w, u32 delta)
  * Called after processing group of packets.
  * but all westwood needs is the last sample of srtt.
  */
-static void tcp_westwood_pkts_acked(struct sock *sk, u32 cnt, s32 rtt)
+static void tcp_westwood_pkts_acked(struct sock *sk,
+				    const struct ack_sample *sample)
 {
 	struct westwood *w = inet_csk_ca(sk);
 
-	if (rtt > 0)
-		w->rtt = usecs_to_jiffies(rtt);
+	if (sample->rtt_us > 0)
+		w->rtt = usecs_to_jiffies(sample->rtt_us);
 }
 
 /*
diff --git a/net/ipv4/tcp_yeah.c b/net/ipv4/tcp_yeah.c
index 3e6a472..028eb04 100644
--- a/net/ipv4/tcp_yeah.c
+++ b/net/ipv4/tcp_yeah.c
@@ -56,15 +56,16 @@ static void tcp_yeah_init(struct sock *sk)
 	tp->snd_cwnd_clamp = min_t(u32, tp->snd_cwnd_clamp, 0xffffffff/128);
 }
 
-static void tcp_yeah_pkts_acked(struct sock *sk, u32 pkts_acked, s32 rtt_us)
+static void tcp_yeah_pkts_acked(struct sock *sk,
+				const struct ack_sample *sample)
 {
 	const struct inet_connection_sock *icsk = inet_csk(sk);
 	struct yeah *yeah = inet_csk_ca(sk);
 
 	if (icsk->icsk_ca_state == TCP_CA_Open)
-		yeah->pkts_acked = pkts_acked;
+		yeah->pkts_acked = sample->pkts_acked;
 
-	tcp_vegas_pkts_acked(sk, pkts_acked, rtt_us);
+	tcp_vegas_pkts_acked(sk, sample);
 }
 
 static void tcp_yeah_cong_avoid(struct sock *sk, u32 ack, u32 acked)
-- 
2.8.0.rc2

^ permalink raw reply related

* Re: [PATCH v3 net-next 00/11] ipv6: Enable GUEoIPv6 and more fixes for v6 tunneling
From: Tom Herbert @ 2016-05-09 21:37 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: David Miller, Netdev, Kernel Team
In-Reply-To: <CAKgT0Ucnr=BzOexzHJ8UXSktAU5Ji9nkmt23+gpQvJxpO3ug=Q@mail.gmail.com>

On Mon, May 9, 2016 at 2:35 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Mon, May 9, 2016 at 10:32 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, May 9, 2016 at 9:56 AM, Tom Herbert <tom@herbertland.com> wrote:
>>> On Fri, May 6, 2016 at 8:03 PM, Alexander Duyck
>>> <alexander.duyck@gmail.com> wrote:
>>>> On Fri, May 6, 2016 at 7:11 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>> On Fri, May 6, 2016 at 7:03 PM, Alexander Duyck
>>>>> <alexander.duyck@gmail.com> wrote:
>>>>>> On Fri, May 6, 2016 at 6:57 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>>>> On Fri, May 6, 2016 at 6:09 PM, Alexander Duyck
>>>>>>> <alexander.duyck@gmail.com> wrote:
>>>>>>>> On Fri, May 6, 2016 at 3:11 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>>>>>> This patch set:
>>>>>>>>>   - Fixes GRE6 to process translate flags correctly from configuration
>>>>>>>>>   - Adds support for GSO and GRO for ip6ip6 and ip4ip6
>>>>>>>>>   - Add support for FOU and GUE in IPv6
>>>>>>>>>   - Support GRE, ip6ip6 and ip4ip6 over FOU/GUE
>>>>>>>>>   - Fixes ip6_input to deal with UDP encapsulations
>>>>>>>>>   - Some other minor fixes
>>>>>>>>>
>>>>>>>>> v2:
>>>>>>>>>   - Removed a check of GSO types in MPLS
>>>>>>>>>   - Define GSO type SKB_GSO_IPXIP6 and SKB_GSO_IPXIP4 (based on input
>>>>>>>>>     from Alexander)
>>>>>>>>>   - Don't define GSO types specifally for IP6IP6 and IP4IP6, above
>>>>>>>>>     fix makes that uncessary
>>>>>>>>>   - Don't bother clearing encapsulation flag in UDP tunnel segment
>>>>>>>>>     (another item suggested by Alexander).
>>>>>>>>>
>>>>>>>>> v3:
>>>>>>>>>   - Address some minor comments from Alexander
>>>>>>>>>
>>>>>>>>> Tested:
>>>>>>>>>    Tested a variety of case, but not the full matrix (which is quite
>>>>>>>>>    large now). Most of the obivous cases (e.g. GRE) work fine. Still
>>>>>>>>>    some issues probably with GSO/GRO being effective in all cases.
>>>>>>>>>
>>>>>>>>>     - IPv4/GRE/GUE/IPv6 with RCO
>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>         6616 Mbps
>>>>>>>>>       200 TCP_RR
>>>>>>>>>         1244043 tps
>>>>>>>>>         141/243/446 90/95/99% latencies
>>>>>>>>>         86.61% CPU utilization
>>>>>>>>>     - IPv6/GRE/GUE/IPv6 with RCO
>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>         6940 Mbps
>>>>>>>>>       200 TCP_RR
>>>>>>>>>         1270903 tps
>>>>>>>>>         138/236/440 90/95/99% latencies
>>>>>>>>>         87.51% CPU utilization
>>>>>>>>>
>>>>>>>>>      - IP6IP6
>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>         2576 Mbps
>>>>>>>>>       200 TCP_RR
>>>>>>>>>         498981 tps
>>>>>>>>>         388/498/631 90/95/99% latencies
>>>>>>>>>         19.75% CPU utilization (1 CPU saturated)
>>>>>>>>>
>>>>>>>>>      - IP6IP6/GUE/IPv6 with RCO
>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>         1854 Mbps
>>>>>>>>>       200 TCP_RR
>>>>>>>>>         1233818 tps
>>>>>>>>>         143/244/451 90/95/99% latencies
>>>>>>>>>         87.57 CPU utilization
>>>>>>>>>
>>>>>>>>>      - IP4IP6
>>>>>>>>>       1 TCP_STREAM
>>>>>>>>>       200 TCP_RR
>>>>>>>>>         763774 tps
>>>>>>>>>         250/318/466 90/95/99% latencies
>>>>>>>>>         35.25% CPU utilization (1 CPU saturated)
>>>>>>>>>
>>>>>>>>>      - GRE with keyid
>>>>>>>>>       200 TCP_RR
>>>>>>>>>         744173 tps
>>>>>>>>>         258/332/461 90/95/99% latencies
>>>>>>>>>         34.59% CPU utilization (1 CPU saturated)
>>>>>>>>
>>>>>>>> So I tried testing your patch set and it looks like I cannot get GRE
>>>>>>>> working for any netperf test.  If I pop the patches off it is even
>>>>>>>> worse since it looks like patch 3 fixes some tunnel flags issues, but
>>>>>>>> still doesn't resolve all the issues introduced with b05229f44228
>>>>>>>> ("gre6: Cleanup GREv6 transmit path, call common GRE functions").
>>>>>>>> Reverting the entire patch seems to resolve the issues, but I will try
>>>>>>>> to pick it apart tonight to see if I can find the other issues that
>>>>>>>> weren't addressed in this patch series.
>>>>>>>>
>>>>>>>
>>>>>>> Can you give details about configuration, test you're running, and HW?
>>>>>>
>>>>>> The issue looks like it may be specific to ip6gretap.  I'm running the
>>>>>> test over an i40e adapter, but it shouldn't make much difference.  I'm
>>>>>> thinking it may have something to do with the MTU configuration as
>>>>>> that is one of the things I am noticing has changed between the
>>>>>> working and the broken version of the code.
>>>>>>
>>>>> I'm not seeing any issue with configuring:
>>>>>
>>>>> ip link add name tun8 type ip6gretap remote
>>>>> 2401:db00:20:911a:face:0:27:0 local 2401:db00:20:911a:face:0:25:0 ttl
>>>>> 225
>>>>>
>>>>> MTU issues would not surprise me with IPv6 though. This is part of the
>>>>> area of code that seems drastically different than what IPv4 is doing.
>>>>
>>>> I am also using a key.
>>>>
>>>>         ip link add $name type ip6gretap key $net \
>>>>                 local fec0::1 remote $addr6 ttl 225 dev $PF0
>>>>
>>> I don't see any issue with key enabled.
>>>
>>>> Does the device you are using support any kind of checksum offload for
>>>> inner headers on GRE tunnels?  It looks like if I turn off checksums
>>>
>>> I don't believe so.
>>>
>>>> and correct the MTU I can then send traffic without issues.  I'd say
>>>> that the Tx cleanup probably introduced 3 regressions.  The first one
>>>> you addressed in patch 3 which fixes the flags.  The second being the
>>>> fact that the MTU is wrong, and the third being something that
>>>> apparently broke checksum and maybe segmentation offload for
>>>> ip6gretap.
>>>>
>>> The MTU can be set in place in IPv6 code that doesn't exist in Ipv4. I
>>> am especially wondering about the "if (p->flags & IP6_TNL_F_CAP_XMIT)"
>>> block.
>
> So I think I have figured out the MTU problem.  You need to go through
> and audit the spots where you are using GRE_ flags instead of TUNNEL_
> flags.  Specifically in ip6gre_tnl_link_config you are using GRE_
> flags to test o_flags which is configured to use the TUNNEL_ prefixed
> flags.  When I changed those out then the MTU started coming out
> correct again.  There were a couple other places you were using
> GRE_SEQ where you should have been using TUNNEL_SEQ as well so you
> could probably clean those up and add them to patch 3 of your set that
> was fixing the flags so that they should be TUNNEL_ prefixed.
>
>>>> Really I think the transmit path cleanup should have probably been
>>>> broken down into a set of patches rather than slamming it in all in
>>>> one block.  I can spend some time next week trying to sort it out if
>>>> you don't have any hardware that supports GRE segmentation or checksum
>>>> offload.  If worse comes to worse I will just try breaking the revert
>>>> down into a set of smaller patches so I can figure out exactly which
>>>> change broke things.
>>>>
>>> I am still trying to reproduce.
>>
>> What NICs are you testing with?  Depending on the NIC I might be able
>> to point you in the direction of something that can reproduce the
>> issue.
>>
>> At this point I am thinking it is an issue with a header offset since
>> I believe GSO resets all that and probably corrects the issue.
>
> I'm still doing some digging on my end.  I'm hoping to have this
> figured out by the end of today.
>

Yes, I will be sending a patch shortly for that if you can try it.

Tom

> - Alex

^ 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