Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 07/10] net: qualcomm: rmnet: Add support for RX checksum offload
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:28 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1514341685-11262-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   | 177 ++++++++++++++++++++-
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c    |   2 +
 4 files changed, 192 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 01d876c..0539d99 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..543e423 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,144 @@
 #define RMNET_MAP_DEAGGR_SPACING  64
 #define RMNET_MAP_DEAGGR_HEADROOM (RMNET_MAP_DEAGGR_SPACING / 2)
 
+static u16 *rmnet_map_get_csum_field(unsigned char protocol,
+				     const void *txporthdr)
+{
+	u16 *check = 0;
+
+	switch (protocol) {
+	case IPPROTO_TCP:
+		check = &(((struct tcphdr *)txporthdr)->check);
+		break;
+
+	case IPPROTO_UDP:
+		check = &(((struct udphdr *)txporthdr)->check);
+		break;
+
+	default:
+		check = 0;
+		break;
+	}
+
+	return check;
+}
+
+static int
+rmnet_map_ipv4_dl_csum_trailer(struct sk_buff *skb,
+			       struct rmnet_map_dl_csum_trailer *csum_trailer)
+{
+	u16 ip_pseudo_payload_csum, pseudo_csum, ip_hdr_csum, *csum_field;
+	u16 csum_value, ip_payload_csum, csum_value_final;
+	struct iphdr *ip4h;
+	void *txporthdr;
+
+	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);
+	ip_hdr_csum = ~ip_fast_csum(ip4h, (int)ip4h->ihl);
+	ip_payload_csum = csum16_sub(csum_value, ip_hdr_csum);
+
+	pseudo_csum = ~ntohs(csum_tcpudp_magic(ip4h->saddr, ip4h->daddr,
+			     (u16)(ntohs(ip4h->tot_len) - ip4h->ihl * 4),
+			     (u16)ip4h->protocol, 0));
+	ip_pseudo_payload_csum = csum16_add(ip_payload_csum, pseudo_csum);
+
+	csum_value_final = ~csum16_sub(ip_pseudo_payload_csum,
+				       ntohs(*csum_field));
+
+	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 == 0xFFFF)
+				csum_value_final = ~csum_value_final;
+			break;
+		}
+	}
+
+	if (csum_value_final == ntohs(*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)
+{
+	u16 ip_pseudo_payload_csum, pseudo_csum, ip6_hdr_csum, *csum_field;
+	u16 csum_value, ip6_payload_csum, csum_value_final;
+	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 = ~ntohs(ip_compute_csum(ip6h,
+			      (int)(txporthdr - (void *)(skb->data))));
+	ip6_payload_csum = csum16_sub(csum_value, ip6_hdr_csum);
+
+	length = (ip6h->nexthdr == IPPROTO_UDP) ?
+		 ntohs(((struct udphdr *)txporthdr)->len) :
+		 ntohs(ip6h->payload_len);
+	pseudo_csum = ~ntohs(csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
+			     length, ip6h->nexthdr, 0));
+	ip_pseudo_payload_csum = csum16_add(ip6_payload_csum, pseudo_csum);
+
+	csum_value_final = ~csum16_sub(ip_pseudo_payload_csum,
+				       ntohs(*csum_field));
+
+	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 == 0xFFFF)
+				csum_value_final = ~csum_value_final;
+			break;
+		}
+	}
+
+	if (csum_value_final == ntohs(*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 +207,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 +220,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 +242,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 (!ntohs(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 08/10] net: qualcomm: rmnet: Handle command packets with checksum trailer
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:28 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1514341685-11262-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 06/10] net: qualcomm: rmnet: Define the MAPv4 packet formats
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:28 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1514341685-11262-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..01d876c 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;
+	u16 csum_value;
+} __aligned(1);
+
+struct rmnet_map_ul_csum_header {
+	u16 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 05/10] net: qualcomm: rmnet: Set pacing rate
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:28 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1514341685-11262-1-git-send-email-subashab@codeaurora.org>

With a default pacing rate of 10, the uplink data rate for a single
TCP stream is around 10Mbps. Setting it to 8 increases it to 146Mbps
which is the maximum supported transmit 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 04/10] net: qualcomm: rmnet: Rename ingress data format to data format
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:27 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1514341685-11262-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 03/10] net: qualcomm: rmnet: Remove unused function declaration
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:27 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1514341685-11262-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 02/10] net: qualcomm: rmnet: Remove invalid condition while stamping mux id
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:27 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1514341685-11262-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 01/10] net: qualcomm: rmnet: Remove redundant check when stamping map header
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:27 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1514341685-11262-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 00/10] net: qualcomm: rmnet: Enable csum offloads
From: Subash Abhinov Kasiviswanathan @ 2017-12-27  2:27 UTC (permalink / raw)
  To: davem, netdev; +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

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 rate
  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   | 298 ++++++++++++++++++++-
 .../net/ethernet/qualcomm/rmnet/rmnet_private.h    |   2 +
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c    |   4 +
 8 files changed, 367 insertions(+), 25 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH bpf-next 2/3] libbpf: add error reporting in XDP
From: Alexei Starovoitov @ 2017-12-27  2:27 UTC (permalink / raw)
  To: Eric Leblond; +Cc: netdev, daniel, linux-kernel, ast
In-Reply-To: <20171225221325.9680-3-eric@regit.org>

On Mon, Dec 25, 2017 at 11:13:24PM +0100, Eric Leblond wrote:
> Parse netlink ext attribute to get the error message returned by
> the card.
> 
> Signed-off-by: Eric Leblond <eric@regit.org>
...
> diff --git a/tools/lib/bpf/nlattr.c b/tools/lib/bpf/nlattr.c
> new file mode 100644
> index 000000000000..962de14f74e3
> --- /dev/null
> +++ b/tools/lib/bpf/nlattr.c
> @@ -0,0 +1,188 @@
> +
> +/*
> + * NETLINK      Netlink attributes
> + *
> + *		Authors:	Thomas Graf <tgraf@suug.ch>
> + *				Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> + */
...
> diff --git a/tools/lib/bpf/nlattr.h b/tools/lib/bpf/nlattr.h
> new file mode 100644
> index 000000000000..b95f3e64c14d
> --- /dev/null
> +++ b/tools/lib/bpf/nlattr.h
> @@ -0,0 +1,164 @@
> +#ifndef __NLATTR_H
> +#define __NLATTR_H

Every file in kernel repo has to have SPDX license identifier.
Also note that tools/lib/bpf is LGPL whereas _if_ you're copying
these functions from kernel lib/nlattr.c then it's GPL which we cannot mix.
Probably easier to copy from libnl instead which is LGPL.

^ permalink raw reply

* Re: [RFC PATCH bpf-next v2 4/4] error-injection: Support fault injection framework
From: Alexei Starovoitov @ 2017-12-27  2:12 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Alexei Starovoitov, Josef Bacik, rostedt, mingo, davem, netdev,
	linux-kernel, ast, kernel-team, daniel, linux-btrfs, darrick.wong,
	Josef Bacik, Akinobu Mita
In-Reply-To: <151427450538.32561.2776225740675148782.stgit@devbox>

On Tue, Dec 26, 2017 at 04:48:25PM +0900, Masami Hiramatsu wrote:
> Support in-kernel fault-injection framework via debugfs.
> This allows you to inject a conditional error to specified
> function using debugfs interfaces.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  Documentation/fault-injection/fault-injection.txt |    5 +
>  kernel/Makefile                                   |    1 
>  kernel/fail_function.c                            |  169 +++++++++++++++++++++
>  lib/Kconfig.debug                                 |   10 +
>  4 files changed, 185 insertions(+)
>  create mode 100644 kernel/fail_function.c
> 
> diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt
> index 918972babcd8..6243a588dd71 100644
> --- a/Documentation/fault-injection/fault-injection.txt
> +++ b/Documentation/fault-injection/fault-injection.txt
> @@ -30,6 +30,11 @@ o fail_mmc_request
>    injects MMC data errors on devices permitted by setting
>    debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request
>  
> +o fail_function
> +
> +  injects error return on specific functions by setting debugfs entries
> +  under /sys/kernel/debug/fail_function. No boot option supported.

I like it.
Could you document it a bit better?
In particular retval is configurable, but without an example no one
will be able to figure out how to use it.

I think you can drop RFC tag from the next version of these patches.
Thanks!

^ permalink raw reply

* Re: [RFC PATCH bpf-next v2 3/4] error-injection: Separate error-injection from kprobe
From: Alexei Starovoitov @ 2017-12-27  2:07 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Alexei Starovoitov, Josef Bacik, rostedt, mingo, davem, netdev,
	linux-kernel, ast, kernel-team, daniel, linux-btrfs, darrick.wong,
	Josef Bacik, Akinobu Mita
In-Reply-To: <151427447559.32561.4219243598746615059.stgit@devbox>

On Tue, Dec 26, 2017 at 04:47:55PM +0900, Masami Hiramatsu wrote:
> Since error-injection framework is not limited to be used
> by kprobes, nor bpf. Other kernel subsystems can use it
> freely for checking safeness of error-injection, e.g.
> livepatch, ftrace etc.
> So this separate error-injection framework from kprobes.
> 
> Some differences has been made:
> 
> - "kprobe" word is removed from any APIs/structures.
> - BPF_ALLOW_ERROR_INJECTION() is renamed to
>   ALLOW_ERROR_INJECTION() since it is not limited for BPF too.
> - CONFIG_FUNCTION_ERROR_INJECTION is the config item of this
>   feature. It is automatically enabled if the arch supports
>   error injection feature for kprobe or ftrace etc.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>   Changes in v2:
>    - Fix the override function name to override_function_with_return()
>    - Show only function name in the list, user don't have to care about
>      it's size, since function override only happens at the entry.

looks like nice cleanup.
Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [RFC PATCH bpf-next v2 2/4] tracing/kprobe: bpf: Compare instruction pointer with original one
From: Alexei Starovoitov @ 2017-12-27  2:00 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Alexei Starovoitov, Josef Bacik, rostedt, mingo, davem, netdev,
	linux-kernel, ast, kernel-team, daniel, linux-btrfs, darrick.wong,
	Josef Bacik, Akinobu Mita
In-Reply-To: <151427444611.32561.15006958504436049655.stgit@devbox>

On Tue, Dec 26, 2017 at 04:47:26PM +0900, Masami Hiramatsu wrote:
> Compare instruction pointer with original one on the
> stack instead using per-cpu bpf_kprobe_override flag.
> 
> This patch also consolidates reset_current_kprobe() and
> preempt_enable_no_resched() blocks. Those can be done
> in one place.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  kernel/trace/bpf_trace.c    |    1 -
>  kernel/trace/trace_kprobe.c |   21 +++++++--------------
>  2 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d663660f8392..cefa9b0e396c 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -83,7 +83,6 @@ EXPORT_SYMBOL_GPL(trace_call_bpf);
>  #ifdef CONFIG_BPF_KPROBE_OVERRIDE
>  BPF_CALL_2(bpf_override_return, struct pt_regs *, regs, unsigned long, rc)
>  {
> -	__this_cpu_write(bpf_kprobe_override, 1);
>  	regs_set_return_value(regs, rc);
>  	arch_ftrace_kprobe_override_function(regs);
>  	return 0;
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 265e3e27e8dc..a7c7035963f2 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -42,8 +42,6 @@ struct trace_kprobe {
>  	(offsetof(struct trace_kprobe, tp.args) +	\
>  	(sizeof(struct probe_arg) * (n)))
>  
> -DEFINE_PER_CPU(int, bpf_kprobe_override);
> -
>  static nokprobe_inline bool trace_kprobe_is_return(struct trace_kprobe *tk)
>  {
>  	return tk->rp.handler != NULL;
> @@ -1204,6 +1202,7 @@ kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
>  	int rctx;
>  
>  	if (bpf_prog_array_valid(call)) {
> +		unsigned long orig_ip = instruction_pointer(regs);
>  		int ret;
>  
>  		ret = trace_call_bpf(call, regs);
> @@ -1211,12 +1210,13 @@ kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
>  		/*
>  		 * We need to check and see if we modified the pc of the
>  		 * pt_regs, and if so clear the kprobe and return 1 so that we
> -		 * don't do the instruction skipping.  Also reset our state so
> -		 * we are clean the next pass through.
> +		 * don't do the single stepping.
> +		 * The ftrace kprobe handler leaves it up to us to re-enable
> +		 * preemption here before returning if we've modified the ip.
>  		 */
> -		if (__this_cpu_read(bpf_kprobe_override)) {
> -			__this_cpu_write(bpf_kprobe_override, 0);
> +		if (orig_ip != instruction_pointer(regs)) {
>  			reset_current_kprobe();
> +			preempt_enable_no_resched();

This is great idea.
Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [RFC PATCH bpf-next v2 1/4] tracing/kprobe: bpf: Check error injectable event is on function entry
From: Alexei Starovoitov @ 2017-12-27  1:57 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Alexei Starovoitov, Josef Bacik, rostedt, mingo, davem, netdev,
	linux-kernel, ast, kernel-team, daniel, linux-btrfs, darrick.wong,
	Josef Bacik, Akinobu Mita
In-Reply-To: <151427441954.32561.8731119329264462024.stgit@devbox>

On Tue, Dec 26, 2017 at 04:46:59PM +0900, Masami Hiramatsu wrote:
> Check whether error injectable event is on function entry or not.
> Currently it checks the event is ftrace-based kprobes or not,
> but that is wrong. It should check if the event is on the entry
> of target function. Since error injection will override a function
> to just return with modified return value, that operation must
> be done before the target function starts making stackframe.
> 
> As a side effect, bpf error injection is no need to depend on
> function-tracer. It can work with sw-breakpoint based kprobe
> events too.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  kernel/trace/Kconfig        |    2 --
>  kernel/trace/bpf_trace.c    |    6 +++---
>  kernel/trace/trace_kprobe.c |    8 +++++---
>  kernel/trace/trace_probe.h  |   12 ++++++------
>  4 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index ae3a2d519e50..6400e1bf97c5 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -533,9 +533,7 @@ config FUNCTION_PROFILER
>  config BPF_KPROBE_OVERRIDE
>  	bool "Enable BPF programs to override a kprobed function"
>  	depends on BPF_EVENTS
> -	depends on KPROBES_ON_FTRACE
>  	depends on HAVE_KPROBE_OVERRIDE
> -	depends on DYNAMIC_FTRACE_WITH_REGS
>  	default n
>  	help
>  	 Allows BPF to override the execution of a probed function and
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index f6d2327ecb59..d663660f8392 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -800,11 +800,11 @@ int perf_event_attach_bpf_prog(struct perf_event *event,
>  	int ret = -EEXIST;
>  
>  	/*
> -	 * Kprobe override only works for ftrace based kprobes, and only if they
> -	 * are on the opt-in list.
> +	 * Kprobe override only works if they are on the function entry,
> +	 * and only if they are on the opt-in list.
>  	 */
>  	if (prog->kprobe_override &&
> -	    (!trace_kprobe_ftrace(event->tp_event) ||
> +	    (!trace_kprobe_on_func_entry(event->tp_event) ||
>  	     !trace_kprobe_error_injectable(event->tp_event)))
>  		return -EINVAL;
>  
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 91f4b57dab82..265e3e27e8dc 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -88,13 +88,15 @@ static nokprobe_inline unsigned long trace_kprobe_nhit(struct trace_kprobe *tk)
>  	return nhit;
>  }
>  
> -int trace_kprobe_ftrace(struct trace_event_call *call)
> +bool trace_kprobe_on_func_entry(struct trace_event_call *call)
>  {
>  	struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
> -	return kprobe_ftrace(&tk->rp.kp);
> +
> +	return kprobe_on_func_entry(tk->rp.kp.addr, tk->rp.kp.symbol_name,
> +				    tk->rp.kp.offset);

That would be nice, but did you test this?
My understanding that kprobe will restore all regs and
here we need to override return ip _and_ value.
Could you add a patch with the test the way Josef did
or describe the steps to test this new mode?

^ permalink raw reply

* Re: [PATCH net-next v8 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE
From: Masahiro Yamada @ 2017-12-27  1:49 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: David Miller, netdev, Andrew Lunn, Florian Fainelli, Rob Herring,
	Mark Rutland, linux-arm-kernel, Linux Kernel Mailing List,
	devicetree, Masami Hiramatsu, Jassi Brar
In-Reply-To: <1514164238-28901-2-git-send-email-hayashi.kunihiko@socionext.com>

2017-12-25 10:10 GMT+09:00 Kunihiko Hayashi <hayashi.kunihiko@socionext.com>:
> DT bindings for the AVE ethernet controller found on Socionext's
> UniPhier platforms.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/net/socionext,uniphier-ave4.txt       | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
>
> diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> new file mode 100644
> index 0000000..8b03668
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> @@ -0,0 +1,47 @@
> +* Socionext AVE ethernet controller
> +
> +This describes the devicetree bindings for AVE ethernet controller
> +implemented on Socionext UniPhier SoCs.
> +
> +Required properties:
> + - compatible: Should be
> +       - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
> +       - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
> +       - "socionext,uniphier-ld11-ave4" : for LD11 SoC
> +       - "socionext,uniphier-ld20-ave4" : for LD20 SoC
> + - reg: Address where registers are mapped and size of region.
> + - interrupts: Should contain the MAC interrupt.
> + - phy-mode: See ethernet.txt in the same directory. Allow to choose
> +       "rgmii", "rmii", or "mii" according to the PHY.
> + - phy-handle: Should point to the external phy device.
> +       See ethernet.txt file in the same directory.
> + - clocks: A phandle to the clock for the MAC.
> +
> +Optional properties:
> + - resets: A phandle to the reset control for the MAC.
> + - local-mac-address: See ethernet.txt in the same directory.
> +
> +Required subnode:
> + - mdio: A container for child nodes representing phy nodes.
> +         See phy.txt in the same directory.
> +
> +Example:
> +
> +       ether: ethernet@65000000 {
> +               compatible = "socionext,uniphier-ld20-ave4";
> +               reg = <0x65000000 0x8500>;
> +               interrupts = <0 66 4>;
> +               phy-mode = "rgmii";
> +               phy-handle = <&ethphy>;
> +               clocks = <&sys_clk 6>;
> +               resets = <&sys_rst 6>;
> +               local-mac-address = [00 00 00 00 00 00];
> +
> +               mdio {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       ethphy: ethphy@1 {
> +                               reg = <1>;
> +                       };
> +               };

Andrew Lunn suggested to put a blank line before the "mdio" subnode in v7:
https://patchwork.kernel.org/patch/10127461/

Does it apply to the "ethphy" subnode, too?



Looks like you have a chance for v9.  Please consider it.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* RE: [PATCH net,stable 1/1] net: fec: unmap the xmit buffer that are not transferred by DMA
From: Andy Duan @ 2017-12-27  1:21 UTC (permalink / raw)
  To: Andy Duan, davem@davemloft.net; +Cc: netdev@vger.kernel.org
In-Reply-To: <1513933929-24985-1-git-send-email-fugang.duan@nxp.com>

From: Fugang Duan <fugang.duan@nxp.com> Sent: Friday, December 22, 2017 5:12 PM
>The enet IP only support 32 bit, it will use swiotlb buffer to do dma mapping
>when xmit buffer DMA memory address is bigger than 4G in i.MX platform.
>After stress suspend/resume test, it will print out:
>
>log:
>[12826.352864] fec 5b040000.ethernet: swiotlb buffer is full (sz: 191 bytes)
>[12826.359676] DMA: Out of SW-IOMMU space for 191 bytes at device
>5b040000.ethernet [12826.367110] fec 5b040000.ethernet eth0: Tx DMA
>memory map failed
>
>The issue is that the ready xmit buffers that are dma mapped but DMA still
>don't copy them into fifo, once MAC restart, these DMA buffers are not
>unmapped.
>So it should check the dma mapping buffer and unmap them.
>
>Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
>---
Hi David,

The patch pass two days long time suspend/resume test. Pls apply it to net tree and stable tree.

Thanks.

> drivers/net/ethernet/freescale/fec_main.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index 2d1b065..e17d10b 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -818,6 +818,12 @@ static void fec_enet_bd_init(struct net_device *dev)
> 		for (i = 0; i < txq->bd.ring_size; i++) {
> 			/* Initialize the BD for every fragment in the page. */
> 			bdp->cbd_sc = cpu_to_fec16(0);
>+			if (bdp->cbd_bufaddr &&
>+			    !IS_TSO_HEADER(txq, fec32_to_cpu(bdp-
>>cbd_bufaddr)))
>+				dma_unmap_single(&fep->pdev->dev,
>+						 fec32_to_cpu(bdp-
>>cbd_bufaddr),
>+						 fec16_to_cpu(bdp-
>>cbd_datlen),
>+						 DMA_TO_DEVICE);
> 			if (txq->tx_skbuff[i]) {
> 				dev_kfree_skb_any(txq->tx_skbuff[i]);
> 				txq->tx_skbuff[i] = NULL;
>--
>1.9.1

^ permalink raw reply

* Re: [RFC PATCH v12 0/5] PCI: rockchip: Move PCIe WAKE# handling into pci core
From: Rafael J. Wysocki @ 2017-12-27  0:44 UTC (permalink / raw)
  To: Jeffy Chen
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, shawn.lin-TNX95d0MmH7DzftRWevZcw,
	briannorris-F7+t8E8rja9g9hUCZPvPmw,
	dianders-F7+t8E8rja9g9hUCZPvPmw, Xinming Hu,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Catalin Marinas,
	Kalle Valo, Heiko Stuebner,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Nishant Sarmukadam, Will Deacon, Matthias Kaehlcke,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Ganapathi Bhat, Frank Rowand,
	Amitkumar Karwar,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171226023646.17722-1-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On Tuesday, December 26, 2017 3:36:41 AM CET Jeffy Chen wrote:
> 
> Currently we are handling wake irq in mrvl wifi driver. Move it into
> pci core.
> 
> Tested on my chromebook bob(with cros 4.4 kernel and mrvl wifi).
> 
> 
> Changes in v13:
> Fix compiler error reported by kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Changes in v12:
> Only add irq definitions for PCI devices and rewrite the commit message.
> Enable the wake irq in noirq stage to avoid possible irq storm.
> 
> Changes in v11:
> Address Brian's comments.
> Only support 1-per-device PCIe WAKE# pin as suggested.
> Move to pcie port as Brian suggested.
> 
> Changes in v10:
> Use device_set_wakeup_capable() instead of device_set_wakeup_enable(),
> since dedicated wakeirq will be lost in device_set_wakeup_enable(false).
> 
> Changes in v9:
> Add section for PCI devices and rewrite the commit message.
> Fix check error in .cleanup().
> Move dedicated wakeirq setup to setup() callback and use
> device_set_wakeup_enable() to enable/disable.
> Rewrite the commit message.
> 
> Changes in v8:
> Add optional "pci", and rewrite commit message.
> Add pci-of.c and use platform_pm_ops to handle the PCIe WAKE# signal.
> Rewrite the commit message.
> 
> Changes in v7:
> Move PCIE_WAKE handling into pci core.
> 
> Changes in v6:
> Fix device_init_wake error handling, and add some comments.
> 
> Changes in v5:
> Move to pci.txt
> Rebase.
> Use "wakeup" instead of "wake"
> 
> Changes in v3:
> Fix error handling.
> 
> Changes in v2:
> Use dev_pm_set_dedicated_wake_irq.
> 
> Jeffy Chen (5):
>   dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq
>   of/irq: Adjust of_pci_irq parsing for multiple interrupts
>   mwifiex: Disable wakeup irq handling for pcie
>   PCI / PM: Add support for the PCIe WAKE# signal for OF
>   arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru
> 
>  Documentation/devicetree/bindings/pci/pci.txt | 10 ++++
>  arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 11 ++--
>  drivers/net/wireless/marvell/mwifiex/main.c   |  4 ++
>  drivers/of/of_pci_irq.c                       | 71 +++++++++++++++++++++++--
>  drivers/pci/Makefile                          |  1 +
>  drivers/pci/pci-driver.c                      | 10 ++++
>  drivers/pci/pci-of.c                          | 75 +++++++++++++++++++++++++++
>  include/linux/of_pci.h                        |  9 ++++
>  8 files changed, 183 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/pci/pci-of.c

I'm going to ignore this version till the discussion on the previous one is
over.

Thanks,
Rafael


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net-next v5 1/6] net: tcp: Add trace events for TCP congestion window tracing
From: David Miller @ 2017-12-26 23:51 UTC (permalink / raw)
  To: mhiramat
  Cc: mingo, ian.mcdonald, vyasevich, stephen, rostedt, peterz, tglx,
	linux-kernel, hpa, gerrit, nhorman, dccp, netdev, linux-sctp, sfr
In-Reply-To: <151390833345.13277.9194686808843116185.stgit@devbox>

From: Masami Hiramatsu <mhiramat@kernel.org>
Date: Fri, 22 Dec 2017 11:05:33 +0900

> This adds an event to trace TCP stat variables with
> slightly intrusive trace-event. This uses ftrace/perf
> event log buffer to trace those state, no needs to
> prepare own ring-buffer, nor custom user apps.
> 
> User can use ftrace to trace this event as below;
> 
>   # cd /sys/kernel/debug/tracing
>   # echo 1 > events/tcp/tcp_probe/enable
>   (run workloads)
>   # cat trace
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
 ...
> +	TP_fast_assign(
> +		const struct tcp_sock *tp = tcp_sk(sk);
> +		const struct inet_sock *inet = inet_sk(sk);
> +
> +		memset(__entry->saddr, 0, sizeof(struct sockaddr_in6));
> +		memset(__entry->daddr, 0, sizeof(struct sockaddr_in6));
> +
> +		if (sk->sk_family == AF_INET) {
> +			struct sockaddr_in *v4 = (void *)__entry->saddr;
> +
> +			v4->sin_family = AF_INET;
> +			v4->sin_port = inet->inet_sport;
> +			v4->sin_addr.s_addr = inet->inet_saddr;
> +			v4 = (void *)__entry->daddr;
> +			v4->sin_family = AF_INET;
> +			v4->sin_port = inet->inet_dport;
> +			v4->sin_addr.s_addr = inet->inet_daddr;
> +#if IS_ENABLED(CONFIG_IPV6)
> +		} else if (sk->sk_family == AF_INET6) {

It looks like doing this ifdef test inside of a trace macro is very
undesirable because it upsets sparse.

Please see the following commit which just went into 'net'.

====================
commit 6a6b0b9914e73a8a54253dd5f6f5e5dd5e4a756c
Author: Mat Martineau <mathew.j.martineau@linux.intel.com>
Date:   Thu Dec 21 10:29:09 2017 -0800

    tcp: Avoid preprocessor directives in tracepoint macro args
    
    Using a preprocessor directive to check for CONFIG_IPV6 in the middle of
    a DECLARE_EVENT_CLASS macro's arg list causes sparse to report a series
    of errors:
    
    ./include/trace/events/tcp.h:68:1: error: directive in argument list
    ./include/trace/events/tcp.h:75:1: error: directive in argument list
    ./include/trace/events/tcp.h:144:1: error: directive in argument list
    ./include/trace/events/tcp.h:151:1: error: directive in argument list
    ./include/trace/events/tcp.h:216:1: error: directive in argument list
    ./include/trace/events/tcp.h:223:1: error: directive in argument list
    ./include/trace/events/tcp.h:274:1: error: directive in argument list
    ./include/trace/events/tcp.h:281:1: error: directive in argument list
    
    Once sparse finds an error, it stops printing warnings for the file it
    is checking. This masks any sparse warnings that would normally be
    reported for the core TCP code.
    
    Instead, handle the preprocessor conditionals in a couple of auxiliary
    macros. This also has the benefit of reducing duplicate code.
    
    Cc: David Ahern <dsahern@gmail.com>
    Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 07cccca..ab34c56 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -25,6 +25,35 @@
 		tcp_state_name(TCP_CLOSING),		\
 		tcp_state_name(TCP_NEW_SYN_RECV))
 
+#define TP_STORE_V4MAPPED(__entry, saddr, daddr)		\
+	do {							\
+		struct in6_addr *pin6;				\
+								\
+		pin6 = (struct in6_addr *)__entry->saddr_v6;	\
+		ipv6_addr_set_v4mapped(saddr, pin6);		\
+		pin6 = (struct in6_addr *)__entry->daddr_v6;	\
+		ipv6_addr_set_v4mapped(daddr, pin6);		\
+	} while (0)
+
+#if IS_ENABLED(CONFIG_IPV6)
+#define TP_STORE_ADDRS(__entry, saddr, daddr, saddr6, daddr6)		\
+	do {								\
+		if (sk->sk_family == AF_INET6) {			\
+			struct in6_addr *pin6;				\
+									\
+			pin6 = (struct in6_addr *)__entry->saddr_v6;	\
+			*pin6 = saddr6;					\
+			pin6 = (struct in6_addr *)__entry->daddr_v6;	\
+			*pin6 = daddr6;					\
+		} else {						\
+			TP_STORE_V4MAPPED(__entry, saddr, daddr);	\
+		}							\
+	} while (0)
+#else
+#define TP_STORE_ADDRS(__entry, saddr, daddr, saddr6, daddr6)	\
+	TP_STORE_V4MAPPED(__entry, saddr, daddr)
+#endif
+
 /*
  * tcp event with arguments sk and skb
  *
@@ -50,7 +79,6 @@ DECLARE_EVENT_CLASS(tcp_event_sk_skb,
 
 	TP_fast_assign(
 		struct inet_sock *inet = inet_sk(sk);
-		struct in6_addr *pin6;
 		__be32 *p32;
 
 		__entry->skbaddr = skb;
@@ -65,20 +93,8 @@ DECLARE_EVENT_CLASS(tcp_event_sk_skb,
 		p32 = (__be32 *) __entry->daddr;
 		*p32 =  inet->inet_daddr;
 
-#if IS_ENABLED(CONFIG_IPV6)
-		if (sk->sk_family == AF_INET6) {
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			*pin6 = sk->sk_v6_rcv_saddr;
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			*pin6 = sk->sk_v6_daddr;
-		} else
-#endif
-		{
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
-		}
+		TP_STORE_ADDRS(__entry, inet->inet_saddr, inet->inet_daddr,
+			      sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
 	),
 
 	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c",
@@ -127,7 +143,6 @@ DECLARE_EVENT_CLASS(tcp_event_sk,
 
 	TP_fast_assign(
 		struct inet_sock *inet = inet_sk(sk);
-		struct in6_addr *pin6;
 		__be32 *p32;
 
 		__entry->skaddr = sk;
@@ -141,20 +156,8 @@ DECLARE_EVENT_CLASS(tcp_event_sk,
 		p32 = (__be32 *) __entry->daddr;
 		*p32 =  inet->inet_daddr;
 
-#if IS_ENABLED(CONFIG_IPV6)
-		if (sk->sk_family == AF_INET6) {
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			*pin6 = sk->sk_v6_rcv_saddr;
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			*pin6 = sk->sk_v6_daddr;
-		} else
-#endif
-		{
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
-		}
+		TP_STORE_ADDRS(__entry, inet->inet_saddr, inet->inet_daddr,
+			       sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
 	),
 
 	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c",
@@ -197,7 +200,6 @@ TRACE_EVENT(tcp_set_state,
 
 	TP_fast_assign(
 		struct inet_sock *inet = inet_sk(sk);
-		struct in6_addr *pin6;
 		__be32 *p32;
 
 		__entry->skaddr = sk;
@@ -213,20 +215,8 @@ TRACE_EVENT(tcp_set_state,
 		p32 = (__be32 *) __entry->daddr;
 		*p32 =  inet->inet_daddr;
 
-#if IS_ENABLED(CONFIG_IPV6)
-		if (sk->sk_family == AF_INET6) {
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			*pin6 = sk->sk_v6_rcv_saddr;
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			*pin6 = sk->sk_v6_daddr;
-		} else
-#endif
-		{
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
-		}
+		TP_STORE_ADDRS(__entry, inet->inet_saddr, inet->inet_daddr,
+			       sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
 	),
 
 	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
@@ -256,7 +246,6 @@ TRACE_EVENT(tcp_retransmit_synack,
 
 	TP_fast_assign(
 		struct inet_request_sock *ireq = inet_rsk(req);
-		struct in6_addr *pin6;
 		__be32 *p32;
 
 		__entry->skaddr = sk;
@@ -271,20 +260,8 @@ TRACE_EVENT(tcp_retransmit_synack,
 		p32 = (__be32 *) __entry->daddr;
 		*p32 = ireq->ir_rmt_addr;
 
-#if IS_ENABLED(CONFIG_IPV6)
-		if (sk->sk_family == AF_INET6) {
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			*pin6 = ireq->ir_v6_loc_addr;
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			*pin6 = ireq->ir_v6_rmt_addr;
-		} else
-#endif
-		{
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			ipv6_addr_set_v4mapped(ireq->ir_loc_addr, pin6);
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			ipv6_addr_set_v4mapped(ireq->ir_rmt_addr, pin6);
-		}
+		TP_STORE_ADDRS(__entry, ireq->ir_loc_addr, ireq->ir_rmt_addr,
+			      ireq->ir_v6_loc_addr, ireq->ir_v6_rmt_addr);
 	),
 
 	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c",

^ permalink raw reply related

* Re: [PATCH v5 net-next 0/7] net: ILA notification mechanism and fixes
From: Tom Herbert @ 2017-12-26 23:50 UTC (permalink / raw)
  To: David Miller
  Cc: Linux Kernel Network Developers, Roopa Prabhu, Rohit LastName
In-Reply-To: <20171226.172911.103209790290549015.davem@davemloft.net>

On Tue, Dec 26, 2017 at 2:29 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <tom@quantonium.net>
> Date: Thu, 21 Dec 2017 11:33:25 -0800
>
>> This patch set adds support to get netlink notifications for ILA
>> routes when a route is used.
>>
>> This patch set contains:
>>
>> - General infrastructure for route notifications
>> - The ILA route notification mechanism
>> - Add net to ila build_state
>> - Add flush command to ila_xlat
>> - Fix use of rhashtable for latest fixes
>>
>> Route notifications will be used in conjunction with populating
>> ILA forwarding caches.
>
> Tom, this is just a wolf in sheep's clothing.
>
Dave,

> It's still a cache controllable by external entities.
>
Yep, that's the nature of the problem. In networks of even modest
scale we anticipate that we'll see the number of virtual addresses
(identifiers) far exceed the number of physical hosts. The mapping of
virtual to physical address is not aggregable, so at full we expect
10s of billions of these discrete mappings in a single network. No
single device will be able hold all these mappings, so they'll be
sharded amongst some number of routers. This works fine for
connectivity except that it would be nice to eliminate the triangular
routing by having the source perform encapsulation for destination
itself. So this is the motivation for a working set cache. It is an
optimization, but in networks like 3GPP, it's a big win to eliminate
anchor points in mobility.

> It still therefore has the DoS'ability aspects.
>
True, if implemented without consideration of DOS this is a very bad
thing as proven already by others. However if we know this going in
then DOS'ability can be mitigated or eliminated depending on the rest
of the implementation and architecture, similar to how SYN attacks can
be dealt with.

For example, suppose a device has 10G input link, we want a cache
entry to be usable for at least 30 seconds, and we have no control
over the users on the other side of the link (a typical eNodeB
scenario). That gives a worse case of 19M pps, 585M packets over 30
seconds. Assuming 64 bytes per cache entry that gets us to 37G of
memory needed in the host. That amount of memory is reasonable for a
networking device. Cost of memory should drop over next few years so
10X scaling within ten years seems feasible.

> You can keep reframing this thing you want out there, either by
> explicitly filling the cache in the kernel or doing it via userspace
> responding the netlink events, but it's still the same exact thing
> with the same set of problems.
>
I would point out that the attack surface area using a redirect
mechanism is _way_ less than request/response that was used by LISP or
OVS.

> I'm sorry, but I can't apply this series.  Nor any series that adds a
> DoS'able facility of forwarding/switching/route objects to the
> kernel.
>
Technically, this patch set was just adding route notificates that
facilitate but aren't a requirement for cache management. However, I
do sympathsize with your concerns. Scaling and DOS are precisely the
big problem to overcome in network virtualization and
identifier/locator split.

Happy Holidays!
Tom

^ permalink raw reply

* Re: [PATCH] hv_netvsc: update VF after name has changed.
From: Stephen Hemminger @ 2017-12-26 23:46 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, sthemmin
In-Reply-To: <20171226.171119.1016371414270103500.davem@davemloft.net>

On Tue, 26 Dec 2017 17:11:19 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Tue, 26 Dec 2017 13:51:43 -0800
> 
> > On Tue, 26 Dec 2017 12:25:12 -0500 (EST)
> > David Miller <davem@davemloft.net> wrote:
> >   
> >> From: Stephen Hemminger <stephen@networkplumber.org>
> >> Date: Wed, 20 Dec 2017 14:33:23 -0800
> >>   
> >> > Since commit 6123c66854c1 ("netvsc: delay setup of VF device")
> >> > the automatic bring up of the VF is delayed to allow userspace (udev)
> >> > a chance to rename the device. This delay is problematic because
> >> > it delays boot and may not be long enough for some cases.
> >> > 
> >> > Instead, use the rename can be used to trigger the next step
> >> > in setup to happen immediately.
> >> > 
> >> > The VF initialization sequence now looks like:
> >> >    * hotplug causes VF network device probe to create network device
> >> >    * netvsc notifier joins VF with netvsc and schedules VF to be
> >> >      setup after timer expires
> >> >    * udev in userspace renames device
> >> >    * if netvsc notifier detects rename, it can cancel timer
> >> >      and do immediate setup
> >> > 
> >> > The delay can also be increased to allow for slower rules.
> >> > Still need the delayed work to handle the case where rename is
> >> > not done.
> >> > 
> >> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>    
> >> 
> >> I'm still seriously perplexed by this whole situation.  
> > 
> > See state diagram below
> >    
> >> Why can't you bring up a VF interface simply because of a text string
> >> used to reffered to it?  
> > 
> > VF network device can not have its name changed if up.
> > The network device kernel API does not allow name change of network
> > device that is up.  There was a patch from Vitaly to allow this, but it
> > was deemed to be too risky.
> > 
> >    https://patchwork.ozlabs.org/patch/799646/  
> 
> Thank you for the state diagram.
> 
> I still, unfortunately, don't see the problem.
> 
> If userland changes the name of the VF device, then they will change
> it before bringing the VF device up.

The requirement is to support several unmodified naive distributions.
All of these expect eth0 to be present and should not require configuration or
manipulation of the VF network device. This is why the bonding script method never
worked.

Userland is not responsible for bringing up the VF device. Most distributions leave
unknown devices alone, some use network manager which brings up everything it sees.
The netvsc device is responsible for having the state of the VF device track the state
of the master (netvsc) device.

> 
> Thanks for your patience with me.

No worries, this is not my favorite way of doing this either. The pure VF method used
by some other cloud providers is easier for userspace; but changing the host (and the
hardware FPGA) is not an option. This all just trying to work out
something practical with the API's available.

^ permalink raw reply

* Re: [PATCH v2 bpf-next 10/11] bpf: Add BPF_SOCK_OPS_STATE_CB
From: Alexei Starovoitov @ 2017-12-26 23:23 UTC (permalink / raw)
  To: Lawrence Brakmo
  Cc: netdev, Kernel Team, Blake Matheny, Daniel Borkmann, Eric Dumazet,
	David S. Miller
In-Reply-To: <20171222012101.3899534-11-brakmo@fb.com>

On Thu, Dec 21, 2017 at 05:21:00PM -0800, Lawrence Brakmo wrote:
> Adds support for calling sock_ops BPF program when there is a TCP state
> change. Two arguments are used; one for the old state and another for
> the new state.
> 
> New op: BPF_SOCK_OPS_STATE_CB.
> 
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
>  include/uapi/linux/bpf.h | 4 ++++
>  include/uapi/linux/tcp.h | 1 +
>  net/ipv4/tcp.c           | 2 ++
>  3 files changed, 7 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 415b951..14040e0 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1024,6 +1024,10 @@ enum {
>  					 * Arg1: sequence number of 1st byte
>  					 * Arg2: # segments
>  					 */
> +	BPF_SOCK_OPS_STATE_CB,		/* Called when TCP changes state.
> +					 * Arg1: old_state
> +					 * Arg2: new_state
> +					 */

exposing tcp state to tcp-bpf program means that internal enum
in include/net/tcp_states.h
enum {
        TCP_ESTABLISHED = 1,
        TCP_SYN_SENT,
        TCP_SYN_RECV,
        TCP_FIN_WAIT1,
        TCP_FIN_WAIT2,
...
becomes uapi.
Also since it's only exposed from tcp side, the same sk_state
field used by other protocols is not exposed and things like
DCCP_PASSIVE_CLOSEREQ stay kernel internal.
I think it's ok.
If we would need to add new tcp state we can always add
it to the end of the enum.
Also it's better to make this explicit and move this enum to
uapi/linux/tcp_states.h or ...

An alternative would be to have an array that does state
remapping just to be consumed by bpf program which is ugly
and slow.

Another alternative would be to add:
enum {
        BPF_TCP_ESTABLISHED = 1,
        BPF_TCP_SYN_SENT,
        BPF_TCP_SYN_RECV,
        BPF_TCP_FIN_WAIT1,
to uapi/linux/bpf.h
and have BUILD_BUG_ON(BPF_TCP_ESTABLISHED != TCP_ESTABLISHED);
and in case somebody touches that enum in the middle
the issue will be caught.

I'd like to hear Dave and Eric opinion here.

^ permalink raw reply

* [PATCH net] sfp: fix sfp-bus oops when removing socket/upstream
From: Russell King @ 2017-12-26 23:15 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev

When we remove a socket or upstream, and the other side isn't
registered, we dereference a NULL pointer, causing a kernel oops.
Fix this.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/sfp-bus.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index 8a1b1f4c1b7c..ab64a142b832 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -356,7 +356,8 @@ EXPORT_SYMBOL_GPL(sfp_register_upstream);
 void sfp_unregister_upstream(struct sfp_bus *bus)
 {
 	rtnl_lock();
-	sfp_unregister_bus(bus);
+	if (bus->sfp)
+		sfp_unregister_bus(bus);
 	bus->upstream = NULL;
 	bus->netdev = NULL;
 	rtnl_unlock();
@@ -459,7 +460,8 @@ EXPORT_SYMBOL_GPL(sfp_register_socket);
 void sfp_unregister_socket(struct sfp_bus *bus)
 {
 	rtnl_lock();
-	sfp_unregister_bus(bus);
+	if (bus->netdev)
+		sfp_unregister_bus(bus);
 	bus->sfp_dev = NULL;
 	bus->sfp = NULL;
 	bus->socket_ops = NULL;
-- 
2.7.4

^ permalink raw reply related

* [PATCH net] phylink: ensure we report link down when LOS asserted
From: Russell King @ 2017-12-26 23:15 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev

Although we disable the netdev carrier, we fail to report in the kernel
log that the link went down.  Fix this.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 827f3f92560e..150cd95a6e1e 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1429,9 +1429,8 @@ static void phylink_sfp_link_down(void *upstream)
 	WARN_ON(!lockdep_rtnl_is_held());
 
 	set_bit(PHYLINK_DISABLE_LINK, &pl->phylink_disable_state);
+	queue_work(system_power_efficient_wq, &pl->resolve);
 	flush_work(&pl->resolve);
-
-	netif_carrier_off(pl->netdev);
 }
 
 static void phylink_sfp_link_up(void *upstream)
-- 
2.7.4

^ permalink raw reply related

* Re: BUG warnings in 4.14.9
From: alexander.levin @ 2017-12-26 23:08 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Wei Wang, Martin KaFai Lau, Eric Dumazet, David S. Miller,
	Greg Kroah-Hartman, Chris Rankin, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20171226185955.GA19208@1wt.eu>

On Tue, Dec 26, 2017 at 07:59:55PM +0100, Willy Tarreau wrote:
>Guys,
>
>Chris reported the bug below and confirmed that reverting commit
>9704f81 (ipv6: grab rt->rt6i_ref before allocating pcpu rt) seems to
>have fixed the issue for him. This patch is a94b9367 in mainline.
>
>I personally have no opinion on the patch, just found it because it
>was the only one touching this area between 4.14.8 and 4.14.9 :-)
>
>Should this be reverted or maybe fixed differently ?

Hi Willy,

This seems to be fixed upstream:

commit 951f788a80ff8b6339c5c1ab888b0d4b4352efd8
Author: Eric Dumazet <edumazet@google.com>
Date:   Sun Oct 8 21:07:18 2017 -0700

    ipv6: fix a BUG in rt6_get_pcpu_route()
    
    Ido reported following splat and provided a patch.
    
    [  122.221814] BUG: using smp_processor_id() in preemptible [00000000] code: sshd/2672
    [  122.221845] caller is debug_smp_processor_id+0x17/0x20
    [  122.221866] CPU: 0 PID: 2672 Comm: sshd Not tainted 4.14.0-rc3-idosch-next-custom #639
    [  122.221880] Hardware name: Mellanox Technologies Ltd. MSN2100-CB2FO/SA001017, BIOS 5.6.5 06/07/2016
    [  122.221893] Call Trace:
    [  122.221919]  dump_stack+0xb1/0x10c
    [  122.221946]  ? _atomic_dec_and_lock+0x124/0x124
    [  122.221974]  ? ___ratelimit+0xfe/0x240
    [  122.222020]  check_preemption_disabled+0x173/0x1b0
    [  122.222060]  debug_smp_processor_id+0x17/0x20
    [  122.222083]  ip6_pol_route+0x1482/0x24a0
    ...
    
    I believe we can simplify this code path a bit, since we no longer
    hold a read_lock and need to release it to avoid a dead lock.
    
    By disabling BH, we make sure we'll prevent code re-entry and
    rt6_get_pcpu_route()/rt6_make_pcpu_route() run on the same cpu.
    
    Fixes: 66f5d6ce53e6 ("ipv6: replace rwlock with rcu and spinlock in fib6_table")
    Reported-by: Ido Schimmel <idosch@mellanox.com>
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Tested-by: Ido Schimmel <idosch@mellanox.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Which itself would depend on:

commit d3843fe5fd45be0e04a251a2cc68893c859a31bd
Author: Wei Wang <weiwan@google.com>
Date:   Fri Oct 6 12:06:06 2017 -0700

    ipv6: replace dst_hold() with dst_hold_safe() in routing code

Which applies with a small conflict.

-- 

Thanks,
Sasha

^ permalink raw reply

* Re: [PATCHv4 1/3] dt-bindings: net: Add DT bindings for Socionext Netsec
From: Rob Herring @ 2017-12-26 23:05 UTC (permalink / raw)
  To: Jassi Brar
  Cc: netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Miller, Arnd Bergmann, Andrew Lunn, Ard Biesheuvel,
	Mark Rutland, Masami Hiramatsu, Jassi Brar
In-Reply-To: <1514007911-16674-1-git-send-email-jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Fri, Dec 22, 2017 at 11:45 PM,  <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> This patch adds documentation for Device-Tree bindings for the
> Socionext NetSec Controller driver.
>
> Signed-off-by: Jassi Brar <jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  .../devicetree/bindings/net/socionext-netsec.txt   | 55 ++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/socionext-netsec.txt

One other comment besides issues discussed on v2...

> diff --git a/Documentation/devicetree/bindings/net/socionext-netsec.txt b/Documentation/devicetree/bindings/net/socionext-netsec.txt
> new file mode 100644
> index 0000000..adc7bfa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext-netsec.txt
> @@ -0,0 +1,55 @@
> +* Socionext NetSec Ethernet Controller IP
> +
> +Required properties:
> +- compatible: Should be "socionext,synquacer-netsec"
> +- reg: Address and length of the control register area, followed by the
> +       address and length of the EEPROM holding the MAC address and
> +       microengine firmware
> +- interrupts: Should contain ethernet controller interrupt
> +- clocks: phandle to the PHY reference clock, and any other clocks to be
> +          switched by runtime_pm
> +- clock-names: Required only if more than a single clock is listed in 'clocks'.
> +               The PHY reference clock must be named 'phy_refclk'
> +- phy-mode: See ethernet.txt file in the same directory
> +- phy-handle: See ethernet.txt in the same directory.
> +
> +- mdio device tree subnode: When the Netsec has a phy connected to its local
> +               mdio, there must be device tree subnode with the following
> +               required properties:
> +
> +       - #address-cells: Must be <1>.
> +       - #size-cells: Must be <0>.
> +
> +       For each phy on the mdio bus, there must be a node with the following
> +       fields:
> +       - compatible: Refer to phy.txt
> +       - reg: phy id used to communicate to phy.
> +
> +Optional properties: (See ethernet.txt file in the same directory)
> +- dma-coherent: Boolean property, must only be present if memory
> +       accesses performed by the device are cache coherent.
> +- local-mac-address: See ethernet.txt in the same directory.
> +- mac-address: See ethernet.txt in the same directory.
> +- max-speed: See ethernet.txt in the same directory.
> +- max-frame-size: See ethernet.txt in the same directory.
> +
> +Example:
> +       eth0: netsec@522d0000 {

ethernet@...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


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