Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Alan Cox @ 2018-01-07 19:24 UTC (permalink / raw)
  To: James Bottomley
  Cc: David Miller, w, alexei.starovoitov, torvalds, dan.j.williams,
	linux-kernel, linux-arch, ak, arnd, gregkh, peterz, netdev, mingo,
	hpa, tglx
In-Reply-To: <1515345560.3075.7.camel@HansenPartnership.com>

> everyone.  I'm not saying this always happens, but it is reasonable to
> let the iterative pushback see if we can get to better code in this
> case rather than trying to cut it of with the "because *security*"
> argument.
> 

I'm not arguing otherwise - I'd just prefer most users machines are
secure while we have the discussion and while we see what other
architectural tricks people can come up with

Alan

^ permalink raw reply

* [PATCH net-next v3 RESEND 10/10] net: qualcomm: rmnet: Add support for GSO
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

Real devices may support scatter gather(SG), so enable SG on rmnet
devices to use GSO. GSO reduces CPU cycles by 20% for a rate of
146Mpbs for a single stream TCP connection.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index f7f57ce..570a227 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -190,6 +190,7 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
 
 	rmnet_dev->hw_features = NETIF_F_RXCSUM;
 	rmnet_dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+	rmnet_dev->hw_features |= NETIF_F_SG;
 
 	rc = register_netdevice(rmnet_dev);
 	if (!rc) {
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 09/10] net: qualcomm: rmnet: Add support for TX checksum offload
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

TX checksum offload applies to TCP / UDP packets which are not
fragmented using the MAPv4 checksum trailer. The following needs to be
done to have checksum computed in hardware -

1. Set the checksum start offset and inset offset.
2. Set the csum_enabled bit
3. Compute and set 1's complement of partial checksum field in
   transport header.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   |   8 ++
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |   2 +
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 120 +++++++++++++++++++++
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c    |   1 +
 4 files changed, 131 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 3409458..601edec 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -141,11 +141,19 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 	additional_header_len = 0;
 	required_headroom = sizeof(struct rmnet_map_header);
 
+	if (port->data_format & RMNET_EGRESS_FORMAT_MAP_CKSUMV4) {
+		additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
+		required_headroom += additional_header_len;
+	}
+
 	if (skb_headroom(skb) < required_headroom) {
 		if (pskb_expand_head(skb, required_headroom, 0, GFP_KERNEL))
 			goto fail;
 	}
 
+	if (port->data_format & RMNET_EGRESS_FORMAT_MAP_CKSUMV4)
+		rmnet_map_checksum_uplink_packet(skb, orig_dev);
+
 	map_header = rmnet_map_add_map_header(skb, additional_header_len, 0);
 	if (!map_header)
 		goto fail;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index ca9f473..6ce31e2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -89,5 +89,7 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 						  int hdrlen, int pad);
 void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
 int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len);
+void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
+				      struct net_device *orig_dev);
 
 #endif /* _RMNET_MAP_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 881c1dc..c74a6c5 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -171,6 +171,86 @@ static __sum16 *rmnet_map_get_csum_field(unsigned char protocol,
 }
 #endif
 
+static void rmnet_map_complement_ipv4_txporthdr_csum_field(void *iphdr)
+{
+	struct iphdr *ip4h = (struct iphdr *)iphdr;
+	void *txphdr;
+	u16 *csum;
+
+	txphdr = iphdr + ip4h->ihl * 4;
+
+	if (ip4h->protocol == IPPROTO_TCP || ip4h->protocol == IPPROTO_UDP) {
+		csum = (u16 *)rmnet_map_get_csum_field(ip4h->protocol, txphdr);
+		*csum = ~(*csum);
+	}
+}
+
+static void
+rmnet_map_ipv4_ul_csum_header(void *iphdr,
+			      struct rmnet_map_ul_csum_header *ul_header,
+			      struct sk_buff *skb)
+{
+	struct iphdr *ip4h = (struct iphdr *)iphdr;
+	__be16 *hdr = (__be16 *)ul_header, offset;
+
+	offset = htons((__force u16)(skb_transport_header(skb) -
+				     (unsigned char *)iphdr));
+	ul_header->csum_start_offset = offset;
+	ul_header->csum_insert_offset = skb->csum_offset;
+	ul_header->csum_enabled = 1;
+	if (ip4h->protocol == IPPROTO_UDP)
+		ul_header->udp_ip4_ind = 1;
+	else
+		ul_header->udp_ip4_ind = 0;
+
+	/* Changing remaining fields to network order */
+	hdr++;
+	*hdr = htons((__force u16)*hdr);
+
+	skb->ip_summed = CHECKSUM_NONE;
+
+	rmnet_map_complement_ipv4_txporthdr_csum_field(iphdr);
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static void rmnet_map_complement_ipv6_txporthdr_csum_field(void *ip6hdr)
+{
+	struct ipv6hdr *ip6h = (struct ipv6hdr *)ip6hdr;
+	void *txphdr;
+	u16 *csum;
+
+	txphdr = ip6hdr + sizeof(struct ipv6hdr);
+
+	if (ip6h->nexthdr == IPPROTO_TCP || ip6h->nexthdr == IPPROTO_UDP) {
+		csum = (u16 *)rmnet_map_get_csum_field(ip6h->nexthdr, txphdr);
+		*csum = ~(*csum);
+	}
+}
+
+static void
+rmnet_map_ipv6_ul_csum_header(void *ip6hdr,
+			      struct rmnet_map_ul_csum_header *ul_header,
+			      struct sk_buff *skb)
+{
+	__be16 *hdr = (__be16 *)ul_header, offset;
+
+	offset = htons((__force u16)(skb_transport_header(skb) -
+				     (unsigned char *)ip6hdr));
+	ul_header->csum_start_offset = offset;
+	ul_header->csum_insert_offset = skb->csum_offset;
+	ul_header->csum_enabled = 1;
+	ul_header->udp_ip4_ind = 0;
+
+	/* Changing remaining fields to network order */
+	hdr++;
+	*hdr = htons((__force u16)*hdr);
+
+	skb->ip_summed = CHECKSUM_NONE;
+
+	rmnet_map_complement_ipv6_txporthdr_csum_field(ip6hdr);
+}
+#endif
+
 /* Adds MAP header to front of skb->data
  * Padding is calculated and set appropriately in MAP header. Mux ID is
  * initialized to 0.
@@ -281,3 +361,43 @@ int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len)
 
 	return 0;
 }
+
+/* Generates UL checksum meta info header for IPv4 and IPv6 over TCP and UDP
+ * packets that are supported for UL checksum offload.
+ */
+void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
+				      struct net_device *orig_dev)
+{
+	struct rmnet_map_ul_csum_header *ul_header;
+	void *iphdr;
+
+	ul_header = (struct rmnet_map_ul_csum_header *)
+		    skb_push(skb, sizeof(struct rmnet_map_ul_csum_header));
+
+	if (unlikely(!(orig_dev->features &
+		     (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM))))
+		goto sw_csum;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		iphdr = (char *)ul_header +
+			sizeof(struct rmnet_map_ul_csum_header);
+
+		if (skb->protocol == htons(ETH_P_IP)) {
+			rmnet_map_ipv4_ul_csum_header(iphdr, ul_header, skb);
+			return;
+		} else if (skb->protocol == htons(ETH_P_IPV6)) {
+#if IS_ENABLED(CONFIG_IPV6)
+			rmnet_map_ipv6_ul_csum_header(iphdr, ul_header, skb);
+			return;
+#else
+			goto sw_csum;
+#endif
+		}
+	}
+
+sw_csum:
+	ul_header->csum_start_offset = 0;
+	ul_header->csum_insert_offset = 0;
+	ul_header->csum_enabled = 0;
+	ul_header->udp_ip4_ind = 0;
+}
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 879a2e0..f7f57ce 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -189,6 +189,7 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
 		return -EBUSY;
 
 	rmnet_dev->hw_features = NETIF_F_RXCSUM;
+	rmnet_dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 
 	rc = register_netdevice(rmnet_dev);
 	if (!rc) {
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 08/10] net: qualcomm: rmnet: Handle command packets with checksum trailer
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

When using the MAPv4 packet format in conjunction with MAP commands,
a dummy DL checksum trailer will be appended to the packet. Before
this packet is sent out as an ACK, the DL checksum trailer needs to be
removed.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
index 51e6049..6bc328f 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_command.c
@@ -58,11 +58,24 @@ static u8 rmnet_map_do_flow_control(struct sk_buff *skb,
 }
 
 static void rmnet_map_send_ack(struct sk_buff *skb,
-			       unsigned char type)
+			       unsigned char type,
+			       struct rmnet_port *port)
 {
 	struct rmnet_map_control_command *cmd;
 	int xmit_status;
 
+	if (port->data_format & RMNET_INGRESS_FORMAT_MAP_CKSUMV4) {
+		if (skb->len < sizeof(struct rmnet_map_header) +
+		    RMNET_MAP_GET_LENGTH(skb) +
+		    sizeof(struct rmnet_map_dl_csum_trailer)) {
+			kfree_skb(skb);
+			return;
+		}
+
+		skb_trim(skb, skb->len -
+			 sizeof(struct rmnet_map_dl_csum_trailer));
+	}
+
 	skb->protocol = htons(ETH_P_MAP);
 
 	cmd = RMNET_MAP_GET_CMD_START(skb);
@@ -100,5 +113,5 @@ void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port)
 		break;
 	}
 	if (rc == RMNET_MAP_COMMAND_ACK)
-		rmnet_map_send_ack(skb, rc);
+		rmnet_map_send_ack(skb, rc, port);
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 07/10] net: qualcomm: rmnet: Add support for RX checksum offload
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

When using the MAPv4 packet format, receive checksum offload can be
enabled in hardware. The checksum computation over pseudo header is
not offloaded but the rest of the checksum computation over
the payload is offloaded. This applies only for TCP / UDP packets
which are not fragmented.

rmnet validates the TCP/UDP checksum for the packet using the checksum
from the checksum trailer added to the packet by hardware. The
validation performed is as following -

1. Perform 1's complement over the checksum value from the trailer
2. Compute 1's complement checksum over IPv4 / IPv6 header and
   subtracts it from the value from step 1
3. Computes 1's complement checksum over IPv4 / IPv6 pseudo header and
   adds it to the value from step 2
4. Subtracts the checksum value from the TCP / UDP header from the
   value from step 3.
5. Compares the value from step 4 to the checksum value from the
   TCP / UDP header.
6. If the comparison in step 5 succeeds, CHECKSUM_UNNECESSARY is set
   and the packet is passed on to network stack. If there is a
   failure, then the packet is passed on as such without modifying
   the ip_summed field.

The checksum field is also checked for UDP checksum 0 as per RFC 768
and for unexpected TCP checksum of 0.

If checksum offload is disabled when using MAPv4 packet format in
receive path, the packet is queued as is to network stack without
the validations above.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   |  15 +-
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |   4 +-
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 186 ++++++++++++++++++++-
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c    |   2 +
 4 files changed, 201 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 8f8c4f2..3409458 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -66,8 +66,8 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 			    struct rmnet_port *port)
 {
 	struct rmnet_endpoint *ep;
+	u16 len, pad;
 	u8 mux_id;
-	u16 len;
 
 	if (RMNET_MAP_GET_CD_BIT(skb)) {
 		if (port->data_format & RMNET_INGRESS_FORMAT_MAP_COMMANDS)
@@ -77,7 +77,8 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 	}
 
 	mux_id = RMNET_MAP_GET_MUX_ID(skb);
-	len = RMNET_MAP_GET_LENGTH(skb) - RMNET_MAP_GET_PAD(skb);
+	pad = RMNET_MAP_GET_PAD(skb);
+	len = RMNET_MAP_GET_LENGTH(skb) - pad;
 
 	if (mux_id >= RMNET_MAX_LOGICAL_EP)
 		goto free_skb;
@@ -90,8 +91,14 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 
 	/* Subtract MAP header */
 	skb_pull(skb, sizeof(struct rmnet_map_header));
-	skb_trim(skb, len);
 	rmnet_set_skb_proto(skb);
+
+	if (port->data_format & RMNET_INGRESS_FORMAT_MAP_CKSUMV4) {
+		if (!rmnet_map_checksum_downlink_packet(skb, len + pad))
+			skb->ip_summed = CHECKSUM_UNNECESSARY;
+	}
+
+	skb_trim(skb, len);
 	rmnet_deliver_skb(skb);
 	return;
 
@@ -115,7 +122,7 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 	}
 
 	if (port->data_format & RMNET_INGRESS_FORMAT_DEAGGREGATION) {
-		while ((skbn = rmnet_map_deaggregate(skb)) != NULL)
+		while ((skbn = rmnet_map_deaggregate(skb, port)) != NULL)
 			__rmnet_map_ingress_handler(skbn, port);
 
 		consume_skb(skb);
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 50c50cd..ca9f473 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -83,9 +83,11 @@ struct rmnet_map_ul_csum_header {
 #define RMNET_MAP_NO_PAD_BYTES        0
 #define RMNET_MAP_ADD_PAD_BYTES       1
 
-struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb);
+struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
+				      struct rmnet_port *port);
 struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 						  int hdrlen, int pad);
 void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
+int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len);
 
 #endif /* _RMNET_MAP_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 978ce26..881c1dc 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -14,6 +14,9 @@
  */
 
 #include <linux/netdevice.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <net/ip6_checksum.h>
 #include "rmnet_config.h"
 #include "rmnet_map.h"
 #include "rmnet_private.h"
@@ -21,6 +24,153 @@
 #define RMNET_MAP_DEAGGR_SPACING  64
 #define RMNET_MAP_DEAGGR_HEADROOM (RMNET_MAP_DEAGGR_SPACING / 2)
 
+static __sum16 *rmnet_map_get_csum_field(unsigned char protocol,
+					 const void *txporthdr)
+{
+	__sum16 *check = NULL;
+
+	switch (protocol) {
+	case IPPROTO_TCP:
+		check = &(((struct tcphdr *)txporthdr)->check);
+		break;
+
+	case IPPROTO_UDP:
+		check = &(((struct udphdr *)txporthdr)->check);
+		break;
+
+	default:
+		check = NULL;
+		break;
+	}
+
+	return check;
+}
+
+static int
+rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
+			       struct rmnet_map_dl_csum_trailer *csum_trailer)
+{
+	__sum16 *csum_field, csum_temp, pseudo_csum, hdr_csum, ip_payload_csum;
+	u16 csum_value, csum_value_final;
+	struct iphdr *ip4h;
+	void *txporthdr;
+	__be16 addend;
+
+	ip4h = (struct iphdr *)(skb->data);
+	if ((ntohs(ip4h->frag_off) & IP_MF) ||
+	    ((ntohs(ip4h->frag_off) & IP_OFFSET) > 0))
+		return -EOPNOTSUPP;
+
+	txporthdr = skb->data + ip4h->ihl * 4;
+
+	csum_field = rmnet_map_get_csum_field(ip4h->protocol, txporthdr);
+
+	if (!csum_field)
+		return -EPROTONOSUPPORT;
+
+	/* RFC 768 - Skip IPv4 UDP packets where sender checksum field is 0 */
+	if (*csum_field == 0 && ip4h->protocol == IPPROTO_UDP)
+		return 0;
+
+	csum_value = ~ntohs(csum_trailer->csum_value);
+	hdr_csum = ~ip_fast_csum(ip4h, (int)ip4h->ihl);
+	ip_payload_csum = csum16_sub((__force __sum16)csum_value,
+				     (__force __be16)hdr_csum);
+
+	pseudo_csum = ~csum_tcpudp_magic(ip4h->saddr, ip4h->daddr,
+					 ntohs(ip4h->tot_len) - ip4h->ihl * 4,
+					 ip4h->protocol, 0);
+	addend = (__force __be16)ntohs((__force __be16)pseudo_csum);
+	pseudo_csum = csum16_add(ip_payload_csum, addend);
+
+	addend = (__force __be16)ntohs((__force __be16)*csum_field);
+	csum_temp = ~csum16_sub(pseudo_csum, addend);
+	csum_value_final = (__force u16)csum_temp;
+
+	if (unlikely(csum_value_final == 0)) {
+		switch (ip4h->protocol) {
+		case IPPROTO_UDP:
+			/* RFC 768 - DL4 1's complement rule for UDP csum 0 */
+			csum_value_final = ~csum_value_final;
+			break;
+
+		case IPPROTO_TCP:
+			/* DL4 Non-RFC compliant TCP checksum found */
+			if (*csum_field == (__force __sum16)0xFFFF)
+				csum_value_final = ~csum_value_final;
+			break;
+		}
+	}
+
+	if (csum_value_final == ntohs((__force __be16)*csum_field))
+		return 0;
+	else
+		return -EINVAL;
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static int
+rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
+			       struct rmnet_map_dl_csum_trailer *csum_trailer)
+{
+	__sum16 *csum_field, ip6_payload_csum, pseudo_csum, csum_temp;
+	u16 csum_value, csum_value_final;
+	__be16 ip6_hdr_csum, addend;
+	struct ipv6hdr *ip6h;
+	void *txporthdr;
+	u32 length;
+
+	ip6h = (struct ipv6hdr *)(skb->data);
+
+	txporthdr = skb->data + sizeof(struct ipv6hdr);
+	csum_field = rmnet_map_get_csum_field(ip6h->nexthdr, txporthdr);
+
+	if (!csum_field)
+		return -EPROTONOSUPPORT;
+
+	csum_value = ~ntohs(csum_trailer->csum_value);
+	ip6_hdr_csum = (__force __be16)
+			~ntohs((__force __be16)ip_compute_csum(ip6h,
+			       (int)(txporthdr - (void *)(skb->data))));
+	ip6_payload_csum = csum16_sub((__force __sum16)csum_value,
+				      ip6_hdr_csum);
+
+	length = (ip6h->nexthdr == IPPROTO_UDP) ?
+		 ntohs(((struct udphdr *)txporthdr)->len) :
+		 ntohs(ip6h->payload_len);
+	pseudo_csum = ~(csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
+			     length, ip6h->nexthdr, 0));
+	addend = (__force __be16)ntohs((__force __be16)pseudo_csum);
+	pseudo_csum = csum16_add(ip6_payload_csum, addend);
+
+	addend = (__force __be16)ntohs((__force __be16)*csum_field);
+	csum_temp = ~csum16_sub(pseudo_csum, addend);
+	csum_value_final = (__force u16)csum_temp;
+
+	if (unlikely(csum_value_final == 0)) {
+		switch (ip6h->nexthdr) {
+		case IPPROTO_UDP:
+			/* RFC 2460 section 8.1
+			 * DL6 One's complement rule for UDP checksum 0
+			 */
+			csum_value_final = ~csum_value_final;
+			break;
+
+		case IPPROTO_TCP:
+			/* DL6 Non-RFC compliant TCP checksum found */
+			if (*csum_field == (__force __sum16)0xFFFF)
+				csum_value_final = ~csum_value_final;
+			break;
+		}
+	}
+
+	if (csum_value_final == ntohs((__force __be16)*csum_field))
+		return 0;
+	else
+		return -EINVAL;
+}
+#endif
+
 /* Adds MAP header to front of skb->data
  * Padding is calculated and set appropriately in MAP header. Mux ID is
  * initialized to 0.
@@ -66,7 +216,8 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
  * returned, indicating that there are no more packets to deaggregate. Caller
  * is responsible for freeing the original skb.
  */
-struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
+struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
+				      struct rmnet_port *port)
 {
 	struct rmnet_map_header *maph;
 	struct sk_buff *skbn;
@@ -78,6 +229,9 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
 	maph = (struct rmnet_map_header *)skb->data;
 	packet_len = ntohs(maph->pkt_len) + sizeof(struct rmnet_map_header);
 
+	if (port->data_format & RMNET_INGRESS_FORMAT_MAP_CKSUMV4)
+		packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
+
 	if (((int)skb->len - (int)packet_len) < 0)
 		return NULL;
 
@@ -97,3 +251,33 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
 
 	return skbn;
 }
+
+/* Validates packet checksums. Function takes a pointer to
+ * the beginning of a buffer which contains the IP payload +
+ * padding + checksum trailer.
+ * Only IPv4 and IPv6 are supported along with TCP & UDP.
+ * Fragmented or tunneled packets are not supported.
+ */
+int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len)
+{
+	struct rmnet_map_dl_csum_trailer *csum_trailer;
+
+	if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM)))
+		return -EOPNOTSUPP;
+
+	csum_trailer = (struct rmnet_map_dl_csum_trailer *)(skb->data + len);
+
+	if (!csum_trailer->valid)
+		return -EINVAL;
+
+	if (skb->protocol == htons(ETH_P_IP))
+		return rmnet_map_ipv4_dl_csum_trailer(skb, csum_trailer);
+	else if (skb->protocol == htons(ETH_P_IPV6))
+#if IS_ENABLED(CONFIG_IPV6)
+		return rmnet_map_ipv6_dl_csum_trailer(skb, csum_trailer);
+#else
+		return -EPROTONOSUPPORT;
+#endif
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 5bb29f4..879a2e0 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -188,6 +188,8 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
 	if (rmnet_get_endpoint(port, id))
 		return -EBUSY;
 
+	rmnet_dev->hw_features = NETIF_F_RXCSUM;
+
 	rc = register_netdevice(rmnet_dev);
 	if (!rc) {
 		ep->egress_dev = rmnet_dev;
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 06/10] net: qualcomm: rmnet: Define the MAPv4 packet formats
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

The MAPv4 packet format adds support for RX / TX checksum offload.
For a bi-directional UDP stream at a rate of 570 / 146 Mbps, roughly
10% CPU cycles are saved.

For receive path, there is a checksum trailer appended to the end of
the MAP packet. The valid field indicates if hardware has computed
the checksum. csum_start_offset indicates the offset from the start
of the IP header from which hardware has computed checksum.
csum_length is the number of bytes over which the checksum was
computed and the resulting value is csum_value.

In the transmit path, a header is appended between the end of the MAP
header and the start of the IP packet. csum_start_offset is the offset
in bytes from which hardware will compute the checksum if the
csum_enabled bit is set. udp_ip4_ind indicates if the checksum
value of 0 is valid or not. csum_insert_offset is the offset from the
csum_start_offset where hardware will insert the computed checksum.

The use of this additional packet format for checksum offload is
explained in subsequent patches.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h     | 16 ++++++++++++++++
 drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index ef0eff2..50c50cd 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -47,6 +47,22 @@ struct rmnet_map_header {
 	u16 pkt_len;
 }  __aligned(1);
 
+struct rmnet_map_dl_csum_trailer {
+	u8  reserved1;
+	u8  valid:1;
+	u8  reserved2:7;
+	u16 csum_start_offset;
+	u16 csum_length;
+	__be16 csum_value;
+} __aligned(1);
+
+struct rmnet_map_ul_csum_header {
+	__be16 csum_start_offset;
+	u16 csum_insert_offset:14;
+	u16 udp_ip4_ind:1;
+	u16 csum_enabled:1;
+} __aligned(1);
+
 #define RMNET_MAP_GET_MUX_ID(Y) (((struct rmnet_map_header *) \
 				 (Y)->data)->mux_id)
 #define RMNET_MAP_GET_CD_BIT(Y) (((struct rmnet_map_header *) \
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
index d214280..de0143e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
@@ -21,6 +21,8 @@
 /* Constants */
 #define RMNET_INGRESS_FORMAT_DEAGGREGATION      BIT(0)
 #define RMNET_INGRESS_FORMAT_MAP_COMMANDS       BIT(1)
+#define RMNET_INGRESS_FORMAT_MAP_CKSUMV4        BIT(2)
+#define RMNET_EGRESS_FORMAT_MAP_CKSUMV4         BIT(3)
 
 /* Replace skb->dev to a virtual rmnet device and pass up the stack */
 #define RMNET_EPMODE_VND (1)
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 05/10] net: qualcomm: rmnet: Set pacing shift
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

The real device over which the rmnet devices are installed also
aggregate multiple IP packets and sends them as a single large
aggregate frame to the hardware. This causes degraded throughput
for TCP TX due to bufferbloat.

To overcome this problem, pacing shift value of 8 is set using the
sk_pacing_shift_update() helper. This value was determined based
on experiments with a single stream TCP TX using iperf for a
duration of 30s.

Pacing shift | Observed data rate (Mbps)
          10 | 9
           9 | 140
           8 | 146 (Max link rate)

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 8e1f43a..8f8c4f2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -16,6 +16,7 @@
 #include <linux/netdevice.h>
 #include <linux/netdev_features.h>
 #include <linux/if_arp.h>
+#include <net/sock.h>
 #include "rmnet_private.h"
 #include "rmnet_config.h"
 #include "rmnet_vnd.h"
@@ -204,6 +205,8 @@ void rmnet_egress_handler(struct sk_buff *skb)
 	struct rmnet_priv *priv;
 	u8 mux_id;
 
+	sk_pacing_shift_update(skb->sk, 8);
+
 	orig_dev = skb->dev;
 	priv = netdev_priv(orig_dev);
 	skb->dev = priv->real_dev;
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 04/10] net: qualcomm: rmnet: Rename ingress data format to data format
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

This is done so that we can use this field for both ingress and
egress flags.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c   | 10 +++++-----
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h   |  2 +-
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c |  5 ++---
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index cedacdd..7e7704d 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -143,7 +143,7 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
 			 struct nlattr *tb[], struct nlattr *data[],
 			 struct netlink_ext_ack *extack)
 {
-	int ingress_format = RMNET_INGRESS_FORMAT_DEAGGREGATION;
+	u32 data_format = RMNET_INGRESS_FORMAT_DEAGGREGATION;
 	struct net_device *real_dev;
 	int mode = RMNET_EPMODE_VND;
 	struct rmnet_endpoint *ep;
@@ -185,11 +185,11 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
 		struct ifla_vlan_flags *flags;
 
 		flags = nla_data(data[IFLA_VLAN_FLAGS]);
-		ingress_format = flags->flags & flags->mask;
+		data_format = flags->flags & flags->mask;
 	}
 
-	netdev_dbg(dev, "data format [ingress 0x%08X]\n", ingress_format);
-	port->ingress_data_format = ingress_format;
+	netdev_dbg(dev, "data format [0x%08X]\n", data_format);
+	port->data_format = data_format;
 
 	return 0;
 
@@ -353,7 +353,7 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
 		struct ifla_vlan_flags *flags;
 
 		flags = nla_data(data[IFLA_VLAN_FLAGS]);
-		port->ingress_data_format = flags->flags & flags->mask;
+		port->data_format = flags->flags & flags->mask;
 	}
 
 	return 0;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index 2ea9fe3..00e4634 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -32,7 +32,7 @@ struct rmnet_endpoint {
  */
 struct rmnet_port {
 	struct net_device *dev;
-	u32 ingress_data_format;
+	u32 data_format;
 	u8 nr_rmnet_devs;
 	u8 rmnet_mode;
 	struct hlist_head muxed_ep[RMNET_MAX_LOGICAL_EP];
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index b2d317e3..8e1f43a 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -69,8 +69,7 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 	u16 len;
 
 	if (RMNET_MAP_GET_CD_BIT(skb)) {
-		if (port->ingress_data_format
-		    & RMNET_INGRESS_FORMAT_MAP_COMMANDS)
+		if (port->data_format & RMNET_INGRESS_FORMAT_MAP_COMMANDS)
 			return rmnet_map_command(skb, port);
 
 		goto free_skb;
@@ -114,7 +113,7 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 		skb_push(skb, ETH_HLEN);
 	}
 
-	if (port->ingress_data_format & RMNET_INGRESS_FORMAT_DEAGGREGATION) {
+	if (port->data_format & RMNET_INGRESS_FORMAT_DEAGGREGATION) {
 		while ((skbn = rmnet_map_deaggregate(skb)) != NULL)
 			__rmnet_map_ingress_handler(skbn, port);
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 03/10] net: qualcomm: rmnet: Remove unused function declaration
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

rmnet_map_demultiplex() is only declared but not defined anywhere,
so remove it.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 4df359d..ef0eff2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -67,7 +67,6 @@ struct rmnet_map_header {
 #define RMNET_MAP_NO_PAD_BYTES        0
 #define RMNET_MAP_ADD_PAD_BYTES       1
 
-u8 rmnet_map_demultiplex(struct sk_buff *skb);
 struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb);
 struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 						  int hdrlen, int pad);
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 02/10] net: qualcomm: rmnet: Remove invalid condition while stamping mux id
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

rmnet devices cannot have a mux id of 255. This is validated when
assigning the mux id to the rmnet devices. As a result, checking for
mux id 255 does not apply in egress path.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 0553932..b2d317e3 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -143,10 +143,7 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 	if (!map_header)
 		goto fail;
 
-	if (mux_id == 0xff)
-		map_header->mux_id = 0;
-	else
-		map_header->mux_id = mux_id;
+	map_header->mux_id = mux_id;
 
 	skb->protocol = htons(ETH_P_MAP);
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 01/10] net: qualcomm: rmnet: Remove redundant check when stamping map header
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515350200-29472-1-git-send-email-subashab@codeaurora.org>

We already check the headroom once in rmnet_map_egress_handler(),
so this is not needed.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 86b8c75..978ce26 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -32,9 +32,6 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
 	u32 padding, map_datalen;
 	u8 *padbytes;
 
-	if (skb_headroom(skb) < sizeof(struct rmnet_map_header))
-		return NULL;
-
 	map_datalen = skb->len - hdrlen;
 	map_header = (struct rmnet_map_header *)
 			skb_push(skb, sizeof(struct rmnet_map_header));
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 RESEND 00/10] net: qualcomm: rmnet: Enable csum offloads
From: Subash Abhinov Kasiviswanathan @ 2018-01-07 18:36 UTC (permalink / raw)
  To: davem, netdev, lkp, edumazet; +Cc: Subash Abhinov Kasiviswanathan

This series introduces the MAPv4 packet format for checksum
offload plus some other minor changes.

Patches 1-3 are cleanups.

Patch 4 renames the ingress format to data format so that all data
formats can be configured using this going forward.

Patch 5 uses the pacing helper to improve TCP transmit performance.

Patch 6-9 defines the the MAPv4 for checksum offload for RX and TX.
A new header and trailer format are used as part of MAPv4.
For RX checksum offload, only the 1's complement of the IP payload
portion is computed by hardware. The meta data from RX header is
used to verify the checksum field in the packet. Note that the
IP packet and its field itself is not modified by hardware.
This gives metadata to help with the RX checksum. For TX, the
required metadata is filled up so hardware can compute the
checksum.

Patch 10 enables GSO on rmnet devices

v1->v2: Fix sparse errors reported by kbuild test robot

v2->v3: Update the commit message for Patch 5 based on Eric's comments

Subash Abhinov Kasiviswanathan (10):
  net: qualcomm: rmnet: Remove redundant check when stamping map header
  net: qualcomm: rmnet: Remove invalid condition while stamping mux id
  net: qualcomm: rmnet: Remove unused function declaration
  net: qualcomm: rmnet: Rename ingress data format to data format
  net: qualcomm: rmnet: Set pacing shift
  net: qualcomm: rmnet: Define the MAPv4 packet formats
  net: qualcomm: rmnet: Add support for RX checksum offload
  net: qualcomm: rmnet: Handle command packets with checksum trailer
  net: qualcomm: rmnet: Add support for TX checksum offload
  net: qualcomm: rmnet: Add support for GSO

 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c |  10 +-
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h |   2 +-
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   |  36 ++-
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |  23 +-
 .../ethernet/qualcomm/rmnet/rmnet_map_command.c    |  17 +-
 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   | 309 ++++++++++++++++++++-
 .../net/ethernet/qualcomm/rmnet/rmnet_private.h    |   2 +
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c    |   4 +
 8 files changed, 378 insertions(+), 25 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [iproute2 1/1] tc: Fix filter protocol output
From: Jiri Pirko @ 2018-01-07 18:35 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: stephen, netdev, xiyou.wangcong
In-Reply-To: <1515336208-1304-1-git-send-email-jhs@emojatatu.com>

Sun, Jan 07, 2018 at 03:43:28PM CET, jhs@mojatatu.com wrote:
>From: Jamal Hadi Salim <jhs@mojatatu.com>
>
>Fixes: 249284ff5a44 ("tc: jsonify filter core")
>Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>---
> tc/tc_filter.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/tc/tc_filter.c b/tc/tc_filter.c
>index 545cc3a..546311a 100644
>--- a/tc/tc_filter.c
>+++ b/tc/tc_filter.c
>@@ -276,7 +276,7 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> 		if (!filter_protocol || filter_protocol != f_proto) {
> 			if (f_proto) {
> 				SPRINT_BUF(b1);
>-				print_string(PRINT_JSON, "protocol",
>+				print_string(PRINT_ANY, "protocol",

ha! Thanks!

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Thomas Gleixner @ 2018-01-07 18:31 UTC (permalink / raw)
  To: James Bottomley
  Cc: David Miller, w, gnomes, alexei.starovoitov, torvalds,
	dan.j.williams, linux-kernel, linux-arch, ak, arnd, gregkh,
	peterz, netdev, mingo, hpa
In-Reply-To: <1515345560.3075.7.camel@HansenPartnership.com>

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

On Sun, 7 Jan 2018, James Bottomley wrote:

> On Sat, 2018-01-06 at 20:36 -0500, David Miller wrote:
> > From: Willy Tarreau <w@1wt.eu>
> > Date: Sat, 6 Jan 2018 21:42:29 +0100
> > 
> > > On Sat, Jan 06, 2018 at 06:38:59PM +0000, Alan Cox wrote:
> > >> Normally people who propose security fixes don't have to argue
> > about the
> > >> fact they added 30 clocks to avoid your box being 0wned.
> > > 
> > > In fact it depends, because if a fix makes the system unusable for
> > its
> > > initial purpose, this fix will simply not be deployed at all, which
> > is
> > > the worst that can happen.
> > 
> > +1
> > 
> > I completely agree with Willy and Alexei.
> > 
> > And the scale isn't even accurate, we're talking about at least
> > hundreds upon hundreds of clocks, not 30, if we add an operation
> > whose side effect is to wait for all pending loads to complete.  So
> > yeah this is going to be heavily scrutinized.
> 
> Plus this is the standard kernel code review MO: we've never blindly
> accepted code just because *security* (otherwise we'd have grsec in by
> now).  We use the pushback to get better and more performant code.
>  What often happens is it turns out that the "either security or
> performance" position was a false dichotomy and there is a way of
> fixing stuff that's acceptable (although not usually perfect) for
> everyone.  I'm not saying this always happens, but it is reasonable to
> let the iterative pushback see if we can get to better code in this
> case rather than trying to cut it of with the "because *security*"
> argument.

In principle I agree, though there are a few things to consider:

1) We have not the time to discuss that for the next 6 month

2) Alexei's analyis is purely based on the public information of the google
   zero folks. If it would be complete and the only attack vector all fine.

   If not and I doubt it is, we're going to regret this decision faster
   than we made it and this is not the kind of play field where we can
   afford that.

The whole 'we know better and performance is key' attitude is what led to
this disaster in the first place. We should finaly start to learn.

Can we please stop that and live with the extra cycles for a few month up
to the point where we get more informed answers to all these questions?

Thanks,

	tglx

^ permalink raw reply

* Re: [PATCH net] ipv6: remove null_entry before adding default route
From: Wei Wang @ 2018-01-07 17:57 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: David Miller, Linux Kernel Network Developers, Eric Dumazet
In-Reply-To: <20180107061657.56kxk24uzuvifumv@kafai-mbp>

On Sat, Jan 6, 2018 at 10:16 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Sat, Jan 06, 2018 at 05:41:28PM -0800, Wei Wang wrote:
>> On Fri, Jan 5, 2018 at 11:42 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>> > On Fri, Jan 05, 2018 at 05:38:35PM -0800, Wei Wang wrote:
>> >> From: Wei Wang <weiwan@google.com>
>> >>
>> >> In the current code, when creating a new fib6 table, tb6_root.leaf gets
>> >> initialized to net->ipv6.ip6_null_entry.
>> >> If a default route is being added with rt->rt6i_metric = 0xffffffff,
>> >> fib6_add() will add this route after net->ipv6.ip6_null_entry. As
>> >> null_entry is shared, it could cause problem.
>> >>
>> >> In order to fix it, set fn->leaf to NULL before calling
>> >> fib6_add_rt2node() when trying to add the first default route.
>> >> And reset fn->leaf to null_entry when adding fails or when deleting the
>> >> last default route.
>> >>
>> >> syzkaller reported the following issue which is fixed by this commit:
>> >> =============================
>> >> WARNING: suspicious RCU usage
>> >> 4.15.0-rc5+ #171 Not tainted
>> >> -----------------------------
>> >> net/ipv6/ip6_fib.c:1702 suspicious rcu_dereference_protected() usage!
>> >>
>> >> other info that might help us debug this:
>> >>
>> >> rcu_scheduler_active = 2, debug_locks = 1
>> >> 4 locks held by swapper/0/0:
>> >>  #0:  ((&net->ipv6.ip6_fib_timer)){+.-.}, at: [<00000000d43f631b>] lockdep_copy_map include/linux/lockdep.h:178 [inline]
>> >>  #0:  ((&net->ipv6.ip6_fib_timer)){+.-.}, at: [<00000000d43f631b>] call_timer_fn+0x1c6/0x820 kernel/time/timer.c:1310
>> >>  #1:  (&(&net->ipv6.fib6_gc_lock)->rlock){+.-.}, at: [<000000002ff9d65c>] spin_lock_bh include/linux/spinlock.h:315 [inline]
>> >>  #1:  (&(&net->ipv6.fib6_gc_lock)->rlock){+.-.}, at: [<000000002ff9d65c>] fib6_run_gc+0x9d/0x3c0 net/ipv6/ip6_fib.c:2007
>> >>  #2:  (rcu_read_lock){....}, at: [<0000000091db762d>] __fib6_clean_all+0x0/0x3a0 net/ipv6/ip6_fib.c:1560
>> >>  #3:  (&(&tb->tb6_lock)->rlock){+.-.}, at: [<000000009e503581>] spin_lock_bh include/linux/spinlock.h:315 [inline]
>> >>  #3:  (&(&tb->tb6_lock)->rlock){+.-.}, at: [<000000009e503581>] __fib6_clean_all+0x1d0/0x3a0 net/ipv6/ip6_fib.c:1948
>> >>
>> >> stack backtrace:
>> >> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.15.0-rc5+ #171
>> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>> >> Call Trace:
>> >>  <IRQ>
>> >>  __dump_stack lib/dump_stack.c:17 [inline]
>> >>  dump_stack+0x194/0x257 lib/dump_stack.c:53
>> >>  lockdep_rcu_suspicious+0x123/0x170 kernel/locking/lockdep.c:4585
>> >>  fib6_del+0xcaa/0x11b0 net/ipv6/ip6_fib.c:1701
>> >>  fib6_clean_node+0x3aa/0x4f0 net/ipv6/ip6_fib.c:1892
>> >>  fib6_walk_continue+0x46c/0x8a0 net/ipv6/ip6_fib.c:1815
>> >>  fib6_walk+0x91/0xf0 net/ipv6/ip6_fib.c:1863
>> >>  fib6_clean_tree+0x1e6/0x340 net/ipv6/ip6_fib.c:1933
>> >>  __fib6_clean_all+0x1f4/0x3a0 net/ipv6/ip6_fib.c:1949
>> >>  fib6_clean_all net/ipv6/ip6_fib.c:1960 [inline]
>> >>  fib6_run_gc+0x16b/0x3c0 net/ipv6/ip6_fib.c:2016
>> >>  fib6_gc_timer_cb+0x20/0x30 net/ipv6/ip6_fib.c:2033
>> >>  call_timer_fn+0x228/0x820 kernel/time/timer.c:1320
>> >>  expire_timers kernel/time/timer.c:1357 [inline]
>> >>  __run_timers+0x7ee/0xb70 kernel/time/timer.c:1660
>> >>  run_timer_softirq+0x4c/0xb0 kernel/time/timer.c:1686
>> >>  __do_softirq+0x2d7/0xb85 kernel/softirq.c:285
>> >>  invoke_softirq kernel/softirq.c:365 [inline]
>> >>  irq_exit+0x1cc/0x200 kernel/softirq.c:405
>> >>  exiting_irq arch/x86/include/asm/apic.h:540 [inline]
>> >>  smp_apic_timer_interrupt+0x16b/0x700 arch/x86/kernel/apic/apic.c:1052
>> >>  apic_timer_interrupt+0xa9/0xb0 arch/x86/entry/entry_64.S:904
>> >>  </IRQ>
>> >>
>> >> Reported-by: syzbot <syzkaller@googlegroups.com>
>> >> Fixes: 66f5d6ce53e6 ("ipv6: replace rwlock with rcu and spinlock in fib6_table")
>> >> Signed-off-by: Wei Wang <weiwan@google.com>
>> >> ---
>> >>  net/ipv6/ip6_fib.c | 45 +++++++++++++++++++++++++++++++++++----------
>> >>  1 file changed, 35 insertions(+), 10 deletions(-)
>> >>
>> >> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
>> >> index d11a5578e4f8..37cb4ad1ea29 100644
>> >> --- a/net/ipv6/ip6_fib.c
>> >> +++ b/net/ipv6/ip6_fib.c
>> >> @@ -640,6 +640,11 @@ static struct fib6_node *fib6_add_1(struct net *net,
>> >>                       if (!(fn->fn_flags & RTN_RTINFO)) {
>> >>                               RCU_INIT_POINTER(fn->leaf, NULL);
>> >>                               rt6_release(leaf);
>> >> +                     /* remove null_entry in the root node */
>> >> +                     } else if (fn->fn_flags & RTN_TL_ROOT &&
>> >> +                                rcu_access_pointer(fn->leaf) ==
>> >> +                                net->ipv6.ip6_null_entry) {
>> >> +                             RCU_INIT_POINTER(fn->leaf, NULL);
>> > It seems the reader side could see tb6_root.leaf == NULL after
>> > this change and I think it should be fine?  If it is, instead
>> > of switching betwen NULL and ip6_null_entry, would it be simpler
>> > to always set tb6_root.leaf to NULL until a legit default route
>> > is added?
>> >
>>
>> Agree that reader side could see tb6_root.leaf == NULL and it should
>> be fine cause the lookup code path takes care of NULL case.
>> If we init tb6_root to NULL, it does seem simpler. The only place we
>> need to take care is that fib6_del() should not try to expunge fn if
>> is root node if it does not contain any route. But does it seem weird
>> that tb6_root.fn_flags is marked with RTN_RTINFO without containing
>> any route?
> Good point on the writer's side.  I think we also need to modify
> fib6_add_1() to expect fn->leaf == NULL.  Also, agree that
> RTN_RTINFO needs to be set/clear accordingly.  We also need to
> make changes to the CONFIG_IPV6_SUBTREES......
> It should be doable but requires more changes to break the writer's side
> assumption.  net-next may be a better fit for those changes.
>

Understood. Yes. Agree. I will give it a try on net-next.

> For this patch, I have one inline comment below:
>

>>
>> >>                       }
>> >>
>> >>                       return fn;
>> >> @@ -1270,14 +1275,27 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
>> >>       return err;
>> >>
>> >>  failure:
>> >> -     /* fn->leaf could be NULL if fn is an intermediate node and we
>> >> -      * failed to add the new route to it in both subtree creation
>> >> -      * failure and fib6_add_rt2node() failure case.
>> >> -      * In both cases, fib6_repair_tree() should be called to fix
>> >> +     /* fn->leaf could be NULL if:
>> >> +      * 1. fn is the root node in the table and we fail to add the default
>> >> +      * route to it.
>> >> +      * In this case, we put fn->leaf back to net->ipv6.ip6_null_entry as
>> >> +      * the way the table was created.
>> >> +      * 2. fn is an intermediate node and we failed to add the new
>> >> +      * route to it in both subtree creation failure and fib6_add_rt2node()
>> >> +      * failure case.
>> >> +      * In this case, fib6_repair_tree() should be called to fix
>> >>        * fn->leaf.
>> >>        */
>> >> -     if (fn && !(fn->fn_flags & (RTN_RTINFO|RTN_ROOT)))
>> >> -             fib6_repair_tree(info->nl_net, table, fn);
>> >> +     if (fn) {
>> >> +             if (fn->fn_flags & RTN_TL_ROOT) {
>> >> +                     if (!rcu_access_pointer(fn->leaf)) {
>> >> +                             rcu_assign_pointer(fn->leaf,
>> >> +                                        info->nl_net->ipv6.ip6_null_entry);
>> >> +                     }
>> >> +             } else if (!(fn->fn_flags & (RTN_RTINFO|RTN_ROOT))) {
>> >> +                     fib6_repair_tree(info->nl_net, table, fn);
> fib6_repair_tree() is responsible to ensure intermediate fn->leaf is
> not NULL (or free the fn if possible).  In this case, we want tb6_root.leaf
> to be not NULL.  Can we consolidate the tb6_root.leaf fixing codes to
> fib6_repair_tree() also such that all tree repairing logic is
> in one place?
>

Yes. Good idea. I will send out v2 for this.

>> >> +             }
>> >> +     }
>> >>       /* Always release dst as dst->__refcnt is guaranteed
>> >>        * to be taken before entering this function
>> >>        */
>> >> @@ -1685,11 +1703,18 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
>> >>       }
>> >>       read_unlock(&net->ipv6.fib6_walker_lock);
>> >>
>> >> -     /* If it was last route, expunge its radix tree node */
>> >> +     /* If it was last route:
>> >> +      * 1. For root node, put back null_entry as how the table was created.
>> >> +      * 2. For other nodes, expunge its radix tree node.
>> >> +      */
>> >>       if (!rcu_access_pointer(fn->leaf)) {
>> >> -             fn->fn_flags &= ~RTN_RTINFO;
>> >> -             net->ipv6.rt6_stats->fib_route_nodes--;
>> >> -             fn = fib6_repair_tree(net, table, fn);
>> >> +             if (fn->fn_flags & RTN_TL_ROOT) {
>> >> +                     rcu_assign_pointer(fn->leaf, net->ipv6.ip6_null_entry);
>> >> +             } else {
>> >> +                     fn->fn_flags &= ~RTN_RTINFO;
>> >> +                     net->ipv6.rt6_stats->fib_route_nodes--;
>> >> +                     fn = fib6_repair_tree(net, table, fn);
>> >> +             }
>> >>       }
>> >>
>> >>       fib6_purge_rt(rt, fn, net);
>> >> --
>> >> 2.16.0.rc0.223.g4a4ac83678-goog
>> >>

^ permalink raw reply

* Re: [PATCH net-next 00/18] ipv6: Align nexthop behaviour with IPv4
From: David Ahern @ 2018-01-07 17:20 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-1-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> This set tries to eliminate some differences between IPv4's and IPv6's
> treatment of nexthops. These differences are most likely a side effect
> of IPv6's data structures (specifically 'rt6_info') that incorporate
> both the route and the nexthop and the late addition of ECMP support in
> commit 51ebd3181572 ("ipv6: add support of equal cost multipath
> (ECMP)").
> 
> IPv4 and IPv6 do not react the same to certain netdev events. For
> example, upon carrier change affected IPv4 nexthops are marked using the
> RTNH_F_LINKDOWN flag and the nexthop group is rebalanced accordingly.
> IPv6 on the other hand, does nothing which forces us to perform a
> carrier check during route lookup and dump. This makes it difficult to
> introduce features such as non-equal-cost multipath that are built on
> top of this set [1].
> 
> In addition, when a netdev is put administratively down IPv4 nexthops
> are marked using the RTNH_F_DEAD flag, whereas IPv6 simply flushes all
> the routes using these nexthops. To be consistent with IPv4, multipath
> routes should only be flushed when all nexthops in the group are
> considered dead.
> 
> The first 12 patches introduce non-functional changes that store the
> RTNH_F_DEAD and RTNH_F_LINKDOWN flags in IPv6 routes based on netdev
> events, in a similar fashion to IPv4. This allows us to remove the
> carrier check performed during route lookup and dump.
> 
> The next three patches make sure we only flush a multipath route when
> all of its nexthops are dead.
> 
> Last three patches add test cases for IPv4/IPv6 FIB. These verify that
> both address families react similarly to netdev events.
> 
> Finally, this series also serves as a good first step towards David
> Ahern's goal of treating nexthops as standalone objects [2], as it makes
> the code more in line with IPv4 where the nexthop and the nexthop group
> are separate objects from the route itself.
> 
> 1. https://github.com/idosch/linux/tree/ipv6-nexthops
> 2. http://vger.kernel.org/netconf2017_files/nexthop-objects.pdf
> 

Thanks for working on this - and creating the test cases.

One of many follow on changes that would be beneficial is to remove the
idev dereference in the hot path to check the
ignore_routes_with_linkdown sysctl.

^ permalink raw reply

* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: James Bottomley @ 2018-01-07 17:19 UTC (permalink / raw)
  To: David Miller, w
  Cc: gnomes, alexei.starovoitov, torvalds, dan.j.williams,
	linux-kernel, linux-arch, ak, arnd, gregkh, peterz, netdev, mingo,
	hpa, tglx
In-Reply-To: <20180106.203654.731754766847198456.davem@davemloft.net>

On Sat, 2018-01-06 at 20:36 -0500, David Miller wrote:
> From: Willy Tarreau <w@1wt.eu>
> Date: Sat, 6 Jan 2018 21:42:29 +0100
> 
> > On Sat, Jan 06, 2018 at 06:38:59PM +0000, Alan Cox wrote:
> >> Normally people who propose security fixes don't have to argue
> about the
> >> fact they added 30 clocks to avoid your box being 0wned.
> > 
> > In fact it depends, because if a fix makes the system unusable for
> its
> > initial purpose, this fix will simply not be deployed at all, which
> is
> > the worst that can happen.
> 
> +1
> 
> I completely agree with Willy and Alexei.
> 
> And the scale isn't even accurate, we're talking about at least
> hundreds upon hundreds of clocks, not 30, if we add an operation
> whose side effect is to wait for all pending loads to complete.  So
> yeah this is going to be heavily scrutinized.

Plus this is the standard kernel code review MO: we've never blindly
accepted code just because *security* (otherwise we'd have grsec in by
now).  We use the pushback to get better and more performant code.
 What often happens is it turns out that the "either security or
performance" position was a false dichotomy and there is a way of
fixing stuff that's acceptable (although not usually perfect) for
everyone.  I'm not saying this always happens, but it is reasonable to
let the iterative pushback see if we can get to better code in this
case rather than trying to cut it of with the "because *security*"
argument.

James

^ permalink raw reply

* Re: [PATCH net-next 18/18] selftests: fib_tests: Add test cases for netdev carrier change
From: David Ahern @ 2018-01-07 17:16 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-19-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> Check that IPv4 and IPv6 react the same when the carrier of a netdev is
> toggled. Local routes should not be affected by this, whereas unicast
> routes should.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  tools/testing/selftests/net/fib_tests.sh | 142 +++++++++++++++++++++++++++++++
>  1 file changed, 142 insertions(+)

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next 17/18] selftests: fib_tests: Add test cases for netdev down
From: David Ahern @ 2018-01-07 17:15 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-18-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> Check that IPv4 and IPv6 react the same when a netdev is being put
> administratively down.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  tools/testing/selftests/net/fib_tests.sh | 141 +++++++++++++++++++++++++++++++
>  1 file changed, 141 insertions(+)
> 

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next 16/18] selftests: fib_tests: Add test cases for IPv4/IPv6 FIB
From: David Ahern @ 2018-01-07 17:15 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-17-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> Add test cases to check that IPv4 and IPv6 react to a netdev being
> unregistered as expected.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  tools/testing/selftests/net/Makefile     |   1 +
>  tools/testing/selftests/net/fib_tests.sh | 146 +++++++++++++++++++++++++++++++
>  2 files changed, 147 insertions(+)
>  create mode 100755 tools/testing/selftests/net/fib_tests.sh

Acked-by: David Ahern <dsahern@gmail.com>

FYI: "ip -netns testns" is more efficient than "ip netns exec testns ip".

^ permalink raw reply

* Re: [PATCH net-next 15/18] ipv6: Flush multipath routes when all siblings are dead
From: David Ahern @ 2018-01-07 17:08 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-16-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> By default, IPv6 deletes nexthops from a multipath route when the
> nexthop device is put administratively down. This differs from IPv4
> where the nexthops are kept, but marked with the RTNH_F_DEAD flag. A
> multipath route is flushed when all of its nexthops become dead.
> 
> Align IPv6 with IPv4 and have it conform to the same guidelines.
> 
> In case the multipath route needs to be flushed, its siblings are
> flushed one by one. Otherwise, the nexthops are marked with the
> appropriate flags and the tree walker is instructed to skip all the
> siblings.
> 
> As explained in previous patches, care is taken to update the sernum of
> the affected tree nodes, so as to prevent the use of wrong dst entries.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  net/ipv6/route.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 75 insertions(+), 8 deletions(-)

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next 14/18] ipv6: Take table lock outside of sernum update function
From: David Ahern @ 2018-01-07 17:05 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-15-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> The next patch is going to allow dead routes to remain in the FIB tree
> in certain situations.
> 
> When this happens we need to be sure to bump the sernum of the nodes
> where these are stored so that potential copies cached in sockets are
> invalidated.
> 
> The function that performs this update assumes the table lock is not
> taken when it is invoked, but that will not be the case when it is
> invoked by the tree walker.
> 
> Have the function assume the lock is taken and make the single caller
> take the lock itself.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  net/ipv6/ip6_fib.c | 5 +----
>  net/ipv6/route.c   | 2 ++
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next 13/18] ipv6: Export sernum update function
From: David Ahern @ 2018-01-07 17:04 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-14-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> We are going to allow dead routes to stay in the FIB tree (e.g., when
> they are part of a multipath route, directly connected route with no
> carrier) and revive them when their nexthop device gains carrier or when
> it is put administratively up.
> 
> This is equivalent to the addition of the route to the FIB tree and we
> should therefore take care of updating the sernum of all the parent
> nodes of the node where the route is stored. Otherwise, we risk sockets
> caching and using sub-optimal dst entries.
> 
> Export the function that performs the above, so that it could be invoked
> from fib6_ifup() later on.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  include/net/ip6_fib.h |  1 +
>  net/ipv6/ip6_fib.c    | 11 ++++++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next 12/18] ipv6: Teach tree walker to skip multipath routes
From: David Ahern @ 2018-01-07 17:04 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-13-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> As explained in previous patch, fib6_ifdown() needs to consider the
> state of all the sibling routes when a multipath route is traversed.
> 
> This is done by evaluating all the siblings when the first sibling in a
> multipath route is traversed. If the multipath route does not need to be
> flushed (e.g., not all siblings are dead), then we should just skip the
> multipath route as our work is done.
> 
> Have the tree walker jump to the last sibling when it is determined that
> the multipath route needs to be skipped.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  net/ipv6/ip6_fib.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

Acked-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next 11/18] ipv6: Add explicit flush indication to routes
From: David Ahern @ 2018-01-07 17:03 UTC (permalink / raw)
  To: Ido Schimmel, netdev
  Cc: davem, roopa, nicolas.dichtel, weiwan, kafai, yoshfuji, mlxsw
In-Reply-To: <20180107104518.31693-12-idosch@mellanox.com>

On 1/7/18 3:45 AM, Ido Schimmel wrote:
> When routes that are a part of a multipath route are evaluated by
> fib6_ifdown() in response to NETDEV_DOWN and NETDEV_UNREGISTER events
> the state of their sibling routes is not considered.
> 
> This will change in subsequent patches in order to align IPv6 with
> IPv4's behavior. For example, when the last sibling in a multipath route
> becomes dead, the entire multipath route needs to be removed.
> 
> To prevent the tree walker from re-evaluating all the sibling routes
> each time, we can simply evaluate them once - when the first sibling is
> traversed.
> 
> If we determine the entire multipath route needs to be removed, then the
> 'should_flush' bit is set in all the siblings, which will cause the
> walker to flush them when it traverses them.
> 
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
>  include/net/ip6_fib.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: David Ahern <dsahern@gmail.com>

^ 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