[PATCH 0/6] Drivers: net: hyperv: Enable various offloads

[PATCH 0/6] Drivers: net: hyperv: Enable various offloads

[K. Y. Srinivasan]

This patch set enables both checksum as well as segmentation offload.
As part of this effort I have enabled scatter gather I/O a well.

K. Y. Srinivasan (6):
  Drivers: net: hyperv: Enable scatter gather I/O
  Drivers: net: hyperv: Cleanup the send path
  Drivers: net: hyperv: Enable offloads on the host
  Drivers: net: hyperv: Enable receive side IP checksum offload
  Drivers: net: hyperv: Enable send side checksum offload
  Drivers: net: hyperv: Enable large send offload

 drivers/net/hyperv/hyperv_net.h   |  135 ++++++++++++++-
 drivers/net/hyperv/netvsc_drv.c   |  336 +++++++++++++++++++++++++++++++------
 drivers/net/hyperv/rndis_filter.c |  153 ++++++++++--------
 3 files changed, 501 insertions(+), 123 deletions(-)

-- 
1.7.4.1

[PATCH 1/6] Drivers: net: hyperv: Enable scatter gather I/O

[K. Y. Srinivasan]

Cleanup the code and enable scatter gather I/O.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c |  155 +++++++++++++++++++++++++++++----------
 1 files changed, 116 insertions(+), 39 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 9ef6be9..afa9c53 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -140,21 +140,125 @@ static void netvsc_xmit_completion(void *context)
 		dev_kfree_skb_any(skb);
 }
 
+static u32 fill_pg_buf(struct page *page, u32 offset, u32 len,
+			struct hv_page_buffer *pb)
+{
+	int j = 0;
+	/*
+	 * Deal with compund pages by ignoring unused part
+	 * of the page.
+	 */
+	page += offset  >> PAGE_SHIFT;
+	offset &= ~PAGE_MASK;
+
+	while (len > 0) {
+		unsigned long bytes;
+		bytes = PAGE_SIZE - offset;
+		if (bytes > len)
+			bytes = len;
+		pb[j].pfn = page_to_pfn(page);
+		pb[j].offset = offset;
+		pb[j].len = bytes;
+
+		offset += bytes;
+		len -= bytes;
+
+		if (offset == PAGE_SIZE && len) {
+			page++;
+			offset = 0;
+			j++;
+		}
+	}
+
+	return j + 1;
+}
+
+static void init_page_array(void *hdr, u32 len, struct sk_buff *skb,
+				struct hv_page_buffer *pb)
+{
+	u32 slots_used = 0;
+	char *data = skb->data;
+	int frags = skb_shinfo(skb)->nr_frags;
+	int i;
+
+	/*
+	 * The packet is laid out thus:
+	 * 1. hdr
+	 * 2. skb linear data
+	 * 3. skb fragment data
+	 */
+	if (hdr != NULL) {
+		slots_used += fill_pg_buf(virt_to_page(hdr),
+					offset_in_page(hdr),
+					len, &pb[slots_used]);
+	}
+
+	slots_used += fill_pg_buf(virt_to_page(data),
+				offset_in_page(data),
+				skb_headlen(skb), &pb[slots_used]);
+
+	for (i = 0; i < frags; i++) {
+		skb_frag_t *frag = skb_shinfo(skb)->frags + i;
+
+		slots_used += fill_pg_buf(skb_frag_page(frag),
+					frag->page_offset,
+					skb_frag_size(frag), &pb[slots_used]);
+	}
+}
+
+static int count_skb_frag_slots(struct sk_buff *skb)
+{
+	int i, frags = skb_shinfo(skb)->nr_frags;
+	int pages = 0;
+
+	for (i = 0; i < frags; i++) {
+		skb_frag_t *frag = skb_shinfo(skb)->frags + i;
+		unsigned long size = skb_frag_size(frag);
+		unsigned long offset = frag->page_offset;
+
+		/* Skip unused frames from start of page */
+		offset &= ~PAGE_MASK;
+		pages += PFN_UP(offset + size);
+	}
+	return pages;
+}
+
+static int netvsc_get_slots(struct sk_buff *skb)
+{
+	char *data = skb->data;
+	unsigned int offset = offset_in_page(data);
+	unsigned int len = skb_headlen(skb);
+	int slots;
+	int frag_slots;
+
+	slots = DIV_ROUND_UP(offset + len, PAGE_SIZE);
+	frag_slots = count_skb_frag_slots(skb);
+	return slots + frag_slots;
+}
+
 static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 {
 	struct net_device_context *net_device_ctx = netdev_priv(net);
 	struct hv_netvsc_packet *packet;
 	int ret;
-	unsigned int i, num_pages, npg_data;
+	unsigned int num_data_pages;
 
-	/* Add multipages for skb->data and additional 2 for RNDIS */
-	npg_data = (((unsigned long)skb->data + skb_headlen(skb) - 1)
-		>> PAGE_SHIFT) - ((unsigned long)skb->data >> PAGE_SHIFT) + 1;
-	num_pages = skb_shinfo(skb)->nr_frags + npg_data + 2;
+	/*
+	 * We will atmost need two pages to describe the rndis
+	 * header. We can only transmit MAX_PAGE_BUFFER_COUNT number
+	 * of pages in a single packet.
+	 */
+	num_data_pages = netvsc_get_slots(skb) + 2;
+	if (num_data_pages > MAX_PAGE_BUFFER_COUNT) {
+		netdev_err(net, "Packet too big: %u\n", skb->len);
+		dev_kfree_skb(skb);
+		net->stats.tx_dropped++;
+		return NETDEV_TX_OK;
+	}
 
 	/* Allocate a netvsc packet based on # of frags. */
 	packet = kzalloc(sizeof(struct hv_netvsc_packet) +
-			 (num_pages * sizeof(struct hv_page_buffer)) +
+			 (num_data_pages * sizeof(struct hv_page_buffer)) +
 			 sizeof(struct rndis_message) +
 			 NDIS_VLAN_PPI_SIZE, GFP_ATOMIC);
 	if (!packet) {
@@ -169,44 +273,17 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	packet->vlan_tci = skb->vlan_tci;
 
 	packet->extension = (void *)(unsigned long)packet +
-				sizeof(struct hv_netvsc_packet) +
-				    (num_pages * sizeof(struct hv_page_buffer));
+			sizeof(struct hv_netvsc_packet) +
+			(num_data_pages * sizeof(struct hv_page_buffer));
 
 	/* If the rndis msg goes beyond 1 page, we will add 1 later */
-	packet->page_buf_cnt = num_pages - 1;
+	packet->page_buf_cnt = num_data_pages - 1;
 
 	/* Initialize it from the skb */
 	packet->total_data_buflen = skb->len;
 
 	/* Start filling in the page buffers starting after RNDIS buffer. */
-	packet->page_buf[1].pfn = virt_to_phys(skb->data) >> PAGE_SHIFT;
-	packet->page_buf[1].offset
-		= (unsigned long)skb->data & (PAGE_SIZE - 1);
-	if (npg_data == 1)
-		packet->page_buf[1].len = skb_headlen(skb);
-	else
-		packet->page_buf[1].len = PAGE_SIZE
-			- packet->page_buf[1].offset;
-
-	for (i = 2; i <= npg_data; i++) {
-		packet->page_buf[i].pfn = virt_to_phys(skb->data
-			+ PAGE_SIZE * (i-1)) >> PAGE_SHIFT;
-		packet->page_buf[i].offset = 0;
-		packet->page_buf[i].len = PAGE_SIZE;
-	}
-	if (npg_data > 1)
-		packet->page_buf[npg_data].len = (((unsigned long)skb->data
-			+ skb_headlen(skb) - 1) & (PAGE_SIZE - 1)) + 1;
-
-	/* Additional fragments are after SKB data */
-	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
-		const skb_frag_t *f = &skb_shinfo(skb)->frags[i];
-
-		packet->page_buf[i+npg_data+1].pfn =
-			page_to_pfn(skb_frag_page(f));
-		packet->page_buf[i+npg_data+1].offset = f->page_offset;
-		packet->page_buf[i+npg_data+1].len = skb_frag_size(f);
-	}
+	init_page_array(NULL, 0, skb, &packet->page_buf[1]);
 
 	/* Set the completion routine */
 	packet->completion.send.send_completion = netvsc_xmit_completion;
@@ -451,8 +528,8 @@ static int netvsc_probe(struct hv_device *dev,
 	net->netdev_ops = &device_ops;
 
 	/* TODO: Add GSO and Checksum offload */
-	net->hw_features = 0;
-	net->features = NETIF_F_HW_VLAN_CTAG_TX;
+	net->hw_features = NETIF_F_SG;
+	net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG;
 
 	SET_ETHTOOL_OPS(net, &ethtool_ops);
 	SET_NETDEV_DEV(net, &dev->device);
-- 
1.7.4.1

[PATCH 2/6] Drivers: net: hyperv: Cleanup the send path

[K. Y. Srinivasan]

In preparation for enabling offloads, cleanup the send path.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |    7 +---
 drivers/net/hyperv/netvsc_drv.c   |   87 +++++++++++++++++++++++++++++-------
 drivers/net/hyperv/rndis_filter.c |   66 ----------------------------
 3 files changed, 71 insertions(+), 89 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 39fc230..694bf7c 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -73,7 +73,7 @@ struct hv_netvsc_packet {
 	} completion;
 
 	/* This points to the memory after page_buf */
-	void *extension;
+	struct rndis_message *rndis_msg;
 
 	u32 total_data_buflen;
 	/* Points to the send/receive buffer where the ethernet frame is */
@@ -126,11 +126,6 @@ void rndis_filter_device_remove(struct hv_device *dev);
 int rndis_filter_receive(struct hv_device *dev,
 			struct hv_netvsc_packet *pkt);
 
-
-
-int rndis_filter_send(struct hv_device *dev,
-			struct hv_netvsc_packet *pkt);
-
 int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter);
 int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac);
 
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index afa9c53..e812529 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -128,6 +128,28 @@ static int netvsc_close(struct net_device *net)
 	return ret;
 }
 
+
+static void *init_ppi_data(struct rndis_message *msg, u32 ppi_size,
+				int pkt_type)
+{
+	struct rndis_packet *rndis_pkt;
+	struct rndis_per_packet_info *ppi;
+
+	rndis_pkt = &msg->msg.pkt;
+	rndis_pkt->data_offset += ppi_size;
+
+	ppi = (struct rndis_per_packet_info *)((ulong)rndis_pkt +
+		rndis_pkt->per_pkt_info_offset + rndis_pkt->per_pkt_info_len);
+
+	ppi->size = ppi_size;
+	ppi->type = pkt_type;
+	ppi->ppi_offset = sizeof(struct rndis_per_packet_info);
+
+	rndis_pkt->per_pkt_info_len += ppi_size;
+
+	return ppi;
+}
+
 static void netvsc_xmit_completion(void *context)
 {
 	struct hv_netvsc_packet *packet = (struct hv_netvsc_packet *)context;
@@ -173,7 +195,7 @@ static u32 fill_pg_buf(struct page *page, u32 offset, u32 len,
 	return j + 1;
 }
 
-static void init_page_array(void *hdr, u32 len, struct sk_buff *skb,
+static u32 init_page_array(void *hdr, u32 len, struct sk_buff *skb,
 				struct hv_page_buffer *pb)
 {
 	u32 slots_used = 0;
@@ -204,6 +226,7 @@ static void init_page_array(void *hdr, u32 len, struct sk_buff *skb,
 					frag->page_offset,
 					skb_frag_size(frag), &pb[slots_used]);
 	}
+	return slots_used;
 }
 
 static int count_skb_frag_slots(struct sk_buff *skb)
@@ -241,15 +264,20 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	struct net_device_context *net_device_ctx = netdev_priv(net);
 	struct hv_netvsc_packet *packet;
 	int ret;
-	unsigned int num_data_pages;
+	unsigned int num_data_pgs;
+	struct rndis_message *rndis_msg;
+	struct rndis_packet *rndis_pkt;
+	u32 rndis_msg_size;
+	bool isvlan;
+	struct rndis_per_packet_info *ppi;
 
 	/*
 	 * We will atmost need two pages to describe the rndis
 	 * header. We can only transmit MAX_PAGE_BUFFER_COUNT number
 	 * of pages in a single packet.
 	 */
-	num_data_pages = netvsc_get_slots(skb) + 2;
-	if (num_data_pages > MAX_PAGE_BUFFER_COUNT) {
+	num_data_pgs = netvsc_get_slots(skb) + 2;
+	if (num_data_pgs > MAX_PAGE_BUFFER_COUNT) {
 		netdev_err(net, "Packet too big: %u\n", skb->len);
 		dev_kfree_skb(skb);
 		net->stats.tx_dropped++;
@@ -258,7 +286,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 
 	/* Allocate a netvsc packet based on # of frags. */
 	packet = kzalloc(sizeof(struct hv_netvsc_packet) +
-			 (num_data_pages * sizeof(struct hv_page_buffer)) +
+			 (num_data_pgs * sizeof(struct hv_page_buffer)) +
 			 sizeof(struct rndis_message) +
 			 NDIS_VLAN_PPI_SIZE, GFP_ATOMIC);
 	if (!packet) {
@@ -272,26 +300,51 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 
 	packet->vlan_tci = skb->vlan_tci;
 
-	packet->extension = (void *)(unsigned long)packet +
-			sizeof(struct hv_netvsc_packet) +
-			(num_data_pages * sizeof(struct hv_page_buffer));
-
-	/* If the rndis msg goes beyond 1 page, we will add 1 later */
-	packet->page_buf_cnt = num_data_pages - 1;
-
-	/* Initialize it from the skb */
+	packet->is_data_pkt = true;
 	packet->total_data_buflen = skb->len;
 
-	/* Start filling in the page buffers starting after RNDIS buffer. */
-	init_page_array(NULL, 0, skb, &packet->page_buf[1]);
+	packet->rndis_msg = (struct rndis_message *)((unsigned long)packet +
+				sizeof(struct hv_netvsc_packet) +
+				(num_data_pgs * sizeof(struct hv_page_buffer)));
 
 	/* Set the completion routine */
 	packet->completion.send.send_completion = netvsc_xmit_completion;
 	packet->completion.send.send_completion_ctx = packet;
 	packet->completion.send.send_completion_tid = (unsigned long)skb;
 
-	ret = rndis_filter_send(net_device_ctx->device_ctx,
-				  packet);
+	isvlan = packet->vlan_tci & VLAN_TAG_PRESENT;
+
+	/* Add the rndis header */
+	rndis_msg = packet->rndis_msg;
+	rndis_msg->ndis_msg_type = RNDIS_MSG_PACKET;
+	rndis_msg->msg_len = packet->total_data_buflen;
+	rndis_pkt = &rndis_msg->msg.pkt;
+	rndis_pkt->data_offset = sizeof(struct rndis_packet);
+	rndis_pkt->data_len = packet->total_data_buflen;
+	rndis_pkt->per_pkt_info_offset = sizeof(struct rndis_packet);
+
+	rndis_msg_size = RNDIS_MESSAGE_SIZE(struct rndis_packet);
+
+	if (isvlan) {
+		struct ndis_pkt_8021q_info *vlan;
+
+		rndis_msg_size += NDIS_VLAN_PPI_SIZE;
+		ppi = init_ppi_data(rndis_msg, NDIS_VLAN_PPI_SIZE,
+					IEEE_8021Q_INFO);
+		vlan = (struct ndis_pkt_8021q_info *)((ulong)ppi +
+						ppi->ppi_offset);
+		vlan->vlanid = packet->vlan_tci & VLAN_VID_MASK;
+		vlan->pri = (packet->vlan_tci & VLAN_PRIO_MASK) >>
+				VLAN_PRIO_SHIFT;
+	}
+
+	/* Start filling in the page buffers with the rndis hdr */
+	rndis_msg->msg_len += rndis_msg_size;
+	packet->page_buf_cnt = init_page_array(rndis_msg, rndis_msg_size,
+					skb, &packet->page_buf[0]);
+
+	ret = netvsc_send(net_device_ctx->device_ctx, packet);
+
 	if (ret == 0) {
 		net->stats.tx_bytes += skb->len;
 		net->stats.tx_packets++;
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 6a9f602..dcbf144e 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -910,69 +910,3 @@ int rndis_filter_close(struct hv_device *dev)
 
 	return rndis_filter_close_device(nvdev->extension);
 }
-
-int rndis_filter_send(struct hv_device *dev,
-			     struct hv_netvsc_packet *pkt)
-{
-	struct rndis_message *rndis_msg;
-	struct rndis_packet *rndis_pkt;
-	u32 rndis_msg_size;
-	bool isvlan = pkt->vlan_tci & VLAN_TAG_PRESENT;
-
-	/* Add the rndis header */
-	rndis_msg = (struct rndis_message *)pkt->extension;
-
-	rndis_msg_size = RNDIS_MESSAGE_SIZE(struct rndis_packet);
-	if (isvlan)
-		rndis_msg_size += NDIS_VLAN_PPI_SIZE;
-
-	rndis_msg->ndis_msg_type = RNDIS_MSG_PACKET;
-	rndis_msg->msg_len = pkt->total_data_buflen +
-				      rndis_msg_size;
-
-	rndis_pkt = &rndis_msg->msg.pkt;
-	rndis_pkt->data_offset = sizeof(struct rndis_packet);
-	if (isvlan)
-		rndis_pkt->data_offset += NDIS_VLAN_PPI_SIZE;
-	rndis_pkt->data_len = pkt->total_data_buflen;
-
-	if (isvlan) {
-		struct rndis_per_packet_info *ppi;
-		struct ndis_pkt_8021q_info *vlan;
-
-		rndis_pkt->per_pkt_info_offset = sizeof(struct rndis_packet);
-		rndis_pkt->per_pkt_info_len = NDIS_VLAN_PPI_SIZE;
-
-		ppi = (struct rndis_per_packet_info *)((ulong)rndis_pkt +
-			rndis_pkt->per_pkt_info_offset);
-		ppi->size = NDIS_VLAN_PPI_SIZE;
-		ppi->type = IEEE_8021Q_INFO;
-		ppi->ppi_offset = sizeof(struct rndis_per_packet_info);
-
-		vlan = (struct ndis_pkt_8021q_info *)((ulong)ppi +
-			ppi->ppi_offset);
-		vlan->vlanid = pkt->vlan_tci & VLAN_VID_MASK;
-		vlan->pri = (pkt->vlan_tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
-	}
-
-	pkt->is_data_pkt = true;
-	pkt->page_buf[0].pfn = virt_to_phys(rndis_msg) >> PAGE_SHIFT;
-	pkt->page_buf[0].offset =
-			(unsigned long)rndis_msg & (PAGE_SIZE-1);
-	pkt->page_buf[0].len = rndis_msg_size;
-
-	/* Add one page_buf if the rndis msg goes beyond page boundary */
-	if (pkt->page_buf[0].offset + rndis_msg_size > PAGE_SIZE) {
-		int i;
-		for (i = pkt->page_buf_cnt; i > 1; i--)
-			pkt->page_buf[i] = pkt->page_buf[i-1];
-		pkt->page_buf_cnt++;
-		pkt->page_buf[0].len = PAGE_SIZE - pkt->page_buf[0].offset;
-		pkt->page_buf[1].pfn = virt_to_phys((void *)((ulong)
-			rndis_msg + pkt->page_buf[0].len)) >> PAGE_SHIFT;
-		pkt->page_buf[1].offset = 0;
-		pkt->page_buf[1].len = rndis_msg_size - pkt->page_buf[0].len;
-	}
-
-	return netvsc_send(dev, pkt);
-}
-- 
1.7.4.1

[PATCH 3/6] Drivers: net: hyperv: Enable offloads on the host

[K. Y. Srinivasan]

Prior to enabling guest side offloads, enable the offloads on the host.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |   55 ++++++++++++++++++++++++
 drivers/net/hyperv/rndis_filter.c |   83 +++++++++++++++++++++++++++++++++++++
 2 files changed, 138 insertions(+), 0 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 694bf7c..8bc4e76 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -721,6 +721,61 @@ struct ndis_pkt_8021q_info {
 	};
 };
 
+struct ndis_oject_header {
+	u8 type;
+	u8 revision;
+	u16 size;
+};
+
+#define NDIS_OBJECT_TYPE_DEFAULT	0x80
+#define NDIS_OFFLOAD_PARAMETERS_REVISION_3 3
+#define NDIS_OFFLOAD_PARAMETERS_NO_CHANGE 0
+#define NDIS_OFFLOAD_PARAMETERS_LSOV2_DISABLED 1
+#define NDIS_OFFLOAD_PARAMETERS_LSOV2_ENABLED  2
+#define NDIS_OFFLOAD_PARAMETERS_LSOV1_ENABLED  2
+#define NDIS_OFFLOAD_PARAMETERS_RSC_DISABLED 1
+#define NDIS_OFFLOAD_PARAMETERS_RSC_ENABLED 2
+#define NDIS_OFFLOAD_PARAMETERS_TX_RX_DISABLED 1
+#define NDIS_OFFLOAD_PARAMETERS_TX_ENABLED_RX_DISABLED 2
+#define NDIS_OFFLOAD_PARAMETERS_RX_ENABLED_TX_DISABLED 3
+#define NDIS_OFFLOAD_PARAMETERS_TX_RX_ENABLED 4
+
+/*
+ * New offload OIDs for NDIS 6
+ */
+#define OID_TCP_OFFLOAD_CURRENT_CONFIG 0xFC01020B /* query only */
+#define OID_TCP_OFFLOAD_PARAMETERS 0xFC01020C		/* set only */
+#define OID_TCP_OFFLOAD_HARDWARE_CAPABILITIES 0xFC01020D/* query only */
+#define OID_TCP_CONNECTION_OFFLOAD_CURRENT_CONFIG 0xFC01020E /* query only */
+#define OID_TCP_CONNECTION_OFFLOAD_HARDWARE_CAPABILITIES 0xFC01020F /* query */
+#define OID_OFFLOAD_ENCAPSULATION 0x0101010A /* set/query */
+
+struct ndis_offload_params {
+	struct ndis_oject_header header;
+	u8 ip_v4_csum;
+	u8 tcp_ip_v4_csum;
+	u8 udp_ip_v4_csum;
+	u8 tcp_ip_v6_csum;
+	u8 udp_ip_v6_csum;
+	u8 lso_v1;
+	u8 ip_sec_v1;
+	u8 lso_v2_ipv4;
+	u8 lso_v2_ipv6;
+	u8 tcp_connection_ip_v4;
+	u8 tcp_connection_ip_v6;
+	u32 flags;
+	u8 ip_sec_v2;
+	u8 ip_sec_v2_ip_v4;
+	struct {
+		u8 rsc_ip_v4;
+		u8 rsc_ip_v6;
+	};
+	struct {
+		u8 encapsulated_packet_task_offload;
+		u8 encapsulation_types;
+	};
+};
+
 #define NDIS_VLAN_PPI_SIZE (sizeof(struct rndis_per_packet_info) + \
 		sizeof(struct ndis_pkt_8021q_info))
 
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index dcbf144e..9e257ac 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -627,6 +627,62 @@ cleanup:
 	return ret;
 }
 
+int rndis_filter_set_offload_params(struct hv_device *hdev,
+				struct ndis_offload_params *req_offloads)
+{
+	struct netvsc_device *nvdev = hv_get_drvdata(hdev);
+	struct rndis_device *rdev = nvdev->extension;
+	struct net_device *ndev = nvdev->ndev;
+	struct rndis_request *request;
+	struct rndis_set_request *set;
+	struct ndis_offload_params *offload_params;
+	struct rndis_set_complete *set_complete;
+	u32 extlen = sizeof(struct ndis_offload_params);
+	int ret, t;
+
+	request = get_rndis_request(rdev, RNDIS_MSG_SET,
+		RNDIS_MESSAGE_SIZE(struct rndis_set_request) + extlen);
+	if (!request)
+		return -ENOMEM;
+
+	set = &request->request_msg.msg.set_req;
+	set->oid = OID_TCP_OFFLOAD_PARAMETERS;
+	set->info_buflen = extlen;
+	set->info_buf_offset = sizeof(struct rndis_set_request);
+	set->dev_vc_handle = 0;
+
+	offload_params = (struct ndis_offload_params *)((ulong)set +
+				set->info_buf_offset);
+	*offload_params = *req_offloads;
+	offload_params->header.type = NDIS_OBJECT_TYPE_DEFAULT;
+	offload_params->header.revision = NDIS_OFFLOAD_PARAMETERS_REVISION_3;
+	offload_params->header.size = extlen;
+
+	ret = rndis_filter_send_request(rdev, request);
+	if (ret != 0)
+		goto cleanup;
+
+	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
+	if (t == 0) {
+		netdev_err(ndev, "timeout before we got aOFFLOAD set response...\n");
+		/*
+		 * can't put_rndis_request, since we may still receive a
+		 * send-completion.
+		 */
+		return -EBUSY;
+	} else {
+		set_complete = &request->response_msg.msg.set_complete;
+		if (set_complete->status != RNDIS_STATUS_SUCCESS) {
+			netdev_err(ndev, "Fail to set MAC on host side:0x%x\n",
+				   set_complete->status);
+			ret = -EINVAL;
+		}
+	}
+
+cleanup:
+	put_rndis_request(rdev, request);
+	return ret;
+}
 
 static int rndis_filter_query_device_link_status(struct rndis_device *dev)
 {
@@ -826,6 +882,7 @@ int rndis_filter_device_add(struct hv_device *dev,
 	struct netvsc_device *net_device;
 	struct rndis_device *rndis_device;
 	struct netvsc_device_info *device_info = additional_info;
+	struct ndis_offload_params offloads;
 
 	rndis_device = get_rndis_device();
 	if (!rndis_device)
@@ -865,6 +922,28 @@ int rndis_filter_device_add(struct hv_device *dev,
 
 	memcpy(device_info->mac_adr, rndis_device->hw_mac_adr, ETH_ALEN);
 
+	/*
+	 * Turn on the offloads; the host supports all of the relevant
+	 * offloads.
+	 */
+	memset(&offloads, 0, sizeof(struct ndis_offload_params));
+	/*
+	 * A value of zero means "no change"; now turn on what we
+	 * want.
+	 */
+	offloads.ip_v4_csum = NDIS_OFFLOAD_PARAMETERS_TX_RX_ENABLED;
+	offloads.tcp_ip_v4_csum = NDIS_OFFLOAD_PARAMETERS_TX_RX_ENABLED;
+	offloads.udp_ip_v4_csum = NDIS_OFFLOAD_PARAMETERS_TX_RX_ENABLED;
+	offloads.tcp_ip_v6_csum = NDIS_OFFLOAD_PARAMETERS_TX_RX_ENABLED;
+	offloads.udp_ip_v6_csum = NDIS_OFFLOAD_PARAMETERS_TX_RX_ENABLED;
+	offloads.lso_v2_ipv4 = NDIS_OFFLOAD_PARAMETERS_LSOV2_ENABLED;
+
+
+	ret = rndis_filter_set_offload_params(dev, &offloads);
+	if (ret)
+		goto err_dev_remv;
+
+
 	rndis_filter_query_device_link_status(rndis_device);
 
 	device_info->link_state = rndis_device->link_state;
@@ -874,6 +953,10 @@ int rndis_filter_device_add(struct hv_device *dev,
 		 device_info->link_state ? "down" : "up");
 
 	return ret;
+
+err_dev_remv:
+	rndis_filter_device_remove(dev);
+	return ret;
 }
 
 void rndis_filter_device_remove(struct hv_device *dev)
-- 
1.7.4.1

[PATCH 4/6] Drivers: net: hyperv: Enable receive side IP checksum offload

[K. Y. Srinivasan]

Enable receive side checksum offload.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |   33 ++++++++++++++++++++++++++++++++-
 drivers/net/hyperv/netvsc_drv.c   |   20 ++++++++++++++++----
 drivers/net/hyperv/rndis_filter.c |    4 +++-
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 8bc4e76..faeb746 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -30,6 +30,7 @@
 
 /* Fwd declaration */
 struct hv_netvsc_packet;
+struct ndis_tcp_ip_checksum_info;
 
 /* Represent the xfer page packet which contains 1 or more netvsc packet */
 struct xferpage_packet {
@@ -117,7 +118,8 @@ int netvsc_send(struct hv_device *device,
 void netvsc_linkstatus_callback(struct hv_device *device_obj,
 				unsigned int status);
 int netvsc_recv_callback(struct hv_device *device_obj,
-			struct hv_netvsc_packet *packet);
+			struct hv_netvsc_packet *packet,
+			struct ndis_tcp_ip_checksum_info *csum_info);
 int rndis_filter_open(struct hv_device *dev);
 int rndis_filter_close(struct hv_device *dev);
 int rndis_filter_device_add(struct hv_device *dev,
@@ -776,9 +778,38 @@ struct ndis_offload_params {
 	};
 };
 
+struct ndis_tcp_ip_checksum_info {
+	union {
+		struct {
+			u32 is_ipv4:1;
+			u32 is_ipv6:1;
+			u32 tcp_checksum:1;
+			u32 udp_checksum:1;
+			u32 ip_header_checksum:1;
+			u32 reserved:11;
+			u32 tcp_header_offset:10;
+		} transmit;
+		struct {
+			u32 tcp_checksum_failed:1;
+			u32 udp_checksum_failed:1;
+			u32 ip_checksum_failed:1;
+			u32 tcp_checksum_succeeded:1;
+			u32 udp_checksum_succeeded:1;
+			u32 ip_checksum_succeeded:1;
+			u32 loopback:1;
+			u32 tcp_checksum_value_invalid:1;
+			u32 ip_checksum_value_invalid:1;
+		} receive;
+		u32  value;
+	};
+};
+
 #define NDIS_VLAN_PPI_SIZE (sizeof(struct rndis_per_packet_info) + \
 		sizeof(struct ndis_pkt_8021q_info))
 
+#define NDIS_CSUM_PPI_SIZE (sizeof(struct rndis_per_packet_info) + \
+		sizeof(struct ndis_tcp_ip_checksum_info))
+
 /* Format of Information buffer passed in a SetRequest for the OID */
 /* OID_GEN_RNDIS_CONFIG_PARAMETER. */
 struct rndis_config_parameter_info {
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index e812529..98562fc 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -394,7 +394,8 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
  * "wire" on the specified device.
  */
 int netvsc_recv_callback(struct hv_device *device_obj,
-				struct hv_netvsc_packet *packet)
+				struct hv_netvsc_packet *packet,
+				struct ndis_tcp_ip_checksum_info *csum_info)
 {
 	struct net_device *net;
 	struct sk_buff *skb;
@@ -421,7 +422,18 @@ int netvsc_recv_callback(struct hv_device *device_obj,
 		packet->total_data_buflen);
 
 	skb->protocol = eth_type_trans(skb, net);
-	skb->ip_summed = CHECKSUM_NONE;
+	if (csum_info) {
+		/*
+		 * We only look at the IP checksum here.
+		 * Should we be dropping the packet if checksum
+		 * failed? How do we deal with other checksums - TCP/UDP?
+		 */
+		if (csum_info->receive.ip_checksum_succeeded)
+			skb->ip_summed = CHECKSUM_UNNECESSARY;
+		else
+			skb->ip_summed = CHECKSUM_NONE;
+	}
+
 	if (packet->vlan_tci & VLAN_TAG_PRESENT)
 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
 				       packet->vlan_tci);
@@ -581,8 +593,8 @@ static int netvsc_probe(struct hv_device *dev,
 	net->netdev_ops = &device_ops;
 
 	/* TODO: Add GSO and Checksum offload */
-	net->hw_features = NETIF_F_SG;
-	net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG;
+	net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG;
+	net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM;
 
 	SET_ETHTOOL_OPS(net, &ethtool_ops);
 	SET_NETDEV_DEV(net, &dev->device);
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 9e257ac..9cbe666 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -370,6 +370,7 @@ static void rndis_filter_receive_data(struct rndis_device *dev,
 	struct rndis_packet *rndis_pkt;
 	u32 data_offset;
 	struct ndis_pkt_8021q_info *vlan;
+	struct ndis_tcp_ip_checksum_info *csum_info;
 
 	rndis_pkt = &msg->msg.pkt;
 
@@ -408,7 +409,8 @@ static void rndis_filter_receive_data(struct rndis_device *dev,
 		pkt->vlan_tci = 0;
 	}
 
-	netvsc_recv_callback(dev->net_dev->dev, pkt);
+	csum_info = rndis_get_ppi(rndis_pkt, TCPIP_CHKSUM_PKTINFO);
+	netvsc_recv_callback(dev->net_dev->dev, pkt, csum_info);
 }
 
 int rndis_filter_receive(struct hv_device *dev,
-- 
1.7.4.1

[PATCH 5/6] Drivers: net: hyperv: Enable send side checksum offload

[K. Y. Srinivasan]

Enable send side checksum offload.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c |   70 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 98562fc..e8159a8 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -259,6 +259,36 @@ static int netvsc_get_slots(struct sk_buff *skb)
 	return slots + frag_slots;
 }
 
+bool get_net_transport_info(struct sk_buff *skb, bool *is_v4,
+				bool *is_tcp, bool *is_udp, u32 *trans_off)
+{
+	*is_tcp = false;
+	*is_udp = false;
+
+	if ((eth_hdr(skb)->h_proto != htons(ETH_P_IP)) &&
+		(eth_hdr(skb)->h_proto != htons(ETH_P_IPV6))) {
+		return false;
+	}
+
+	*trans_off = skb_transport_offset(skb);
+
+	if ((eth_hdr(skb)->h_proto == htons(ETH_P_IP))) {
+		struct iphdr *iphdr = ip_hdr(skb);
+		*is_v4 = true;
+		if (iphdr->protocol == IPPROTO_TCP)
+			*is_tcp = true;
+		else if (iphdr->protocol == IPPROTO_UDP)
+			*is_udp = true;
+	} else {
+		*is_v4 = false;
+		if (ipv6_hdr(skb)->nexthdr == IPPROTO_TCP)
+			*is_tcp = true;
+		else if (ipv6_hdr(skb)->nexthdr == IPPROTO_UDP)
+			*is_udp = true;
+	}
+	return true;
+}
+
 static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 {
 	struct net_device_context *net_device_ctx = netdev_priv(net);
@@ -270,6 +300,12 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	u32 rndis_msg_size;
 	bool isvlan;
 	struct rndis_per_packet_info *ppi;
+	struct ndis_tcp_ip_checksum_info *csum_info;
+	int  hdr_offset;
+	bool is_v4;
+	bool is_tcp;
+	bool is_udp;
+
 
 	/*
 	 * We will atmost need two pages to describe the rndis
@@ -338,6 +374,35 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 				VLAN_PRIO_SHIFT;
 	}
 
+	if (!get_net_transport_info(skb, &is_v4, &is_tcp, &is_udp, &hdr_offset))
+		goto do_send;
+	/*
+	 * Setup the sendside checksum offload only if this is not a
+	 * GSO packet.
+	 */
+	if (skb_is_gso(skb))
+		goto do_send;
+
+	rndis_msg_size += NDIS_CSUM_PPI_SIZE;
+	ppi = init_ppi_data(rndis_msg, NDIS_CSUM_PPI_SIZE,
+				TCPIP_CHKSUM_PKTINFO);
+
+	csum_info = (struct ndis_tcp_ip_checksum_info *)((ulong)ppi +
+			ppi->ppi_offset);
+
+	if (is_v4)
+		csum_info->transmit.is_ipv4 = 1;
+	else
+		csum_info->transmit.is_ipv6 = 1;
+
+	if (is_tcp) {
+		csum_info->transmit.tcp_checksum = 1;
+		csum_info->transmit.tcp_header_offset = hdr_offset;
+	} else if (is_udp) {
+		csum_info->transmit.udp_checksum = 1;
+	}
+
+do_send:
 	/* Start filling in the page buffers with the rndis hdr */
 	rndis_msg->msg_len += rndis_msg_size;
 	packet->page_buf_cnt = init_page_array(rndis_msg, rndis_msg_size,
@@ -593,8 +658,9 @@ static int netvsc_probe(struct hv_device *dev,
 	net->netdev_ops = &device_ops;
 
 	/* TODO: Add GSO and Checksum offload */
-	net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG;
-	net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM;
+	net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM;
+	net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM |
+			NETIF_F_IP_CSUM;
 
 	SET_ETHTOOL_OPS(net, &ethtool_ops);
 	SET_NETDEV_DEV(net, &dev->device);
-- 
1.7.4.1

[PATCH 6/6] Drivers: net: hyperv: Enable large send offload

[K. Y. Srinivasan]

Enable segmentation offload.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h |   40 +++++++++++++++++++++++++++++++++++++++
 drivers/net/hyperv/netvsc_drv.c |   38 +++++++++++++++++++++++++++++++++---
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index faeb746..ae5a899 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -742,6 +742,10 @@ struct ndis_oject_header {
 #define NDIS_OFFLOAD_PARAMETERS_RX_ENABLED_TX_DISABLED 3
 #define NDIS_OFFLOAD_PARAMETERS_TX_RX_ENABLED 4
 
+#define NDIS_TCP_LARGE_SEND_OFFLOAD_V2_TYPE	1
+#define NDIS_TCP_LARGE_SEND_OFFLOAD_IPV4	0
+#define NDIS_TCP_LARGE_SEND_OFFLOAD_IPV6	1
+
 /*
  * New offload OIDs for NDIS 6
  */
@@ -804,12 +808,48 @@ struct ndis_tcp_ip_checksum_info {
 	};
 };
 
+struct ndis_tcp_lso_info {
+	union {
+		struct {
+			u32 unused:30;
+			u32 type:1;
+			u32 reserved2:1;
+		} transmit;
+		struct {
+			u32 mss:20;
+			u32 tcp_header_offset:10;
+			u32 type:1;
+			u32 reserved2:1;
+		} lso_v1_transmit;
+		struct {
+			u32 tcp_payload:30;
+			u32 type:1;
+			u32 reserved2:1;
+		} lso_v1_transmit_complete;
+		struct {
+			u32 mss:20;
+			u32 tcp_header_offset:10;
+			u32 type:1;
+			u32 ip_version:1;
+		} lso_v2_transmit;
+		struct {
+			u32 reserved:30;
+			u32 type:1;
+			u32 reserved2:1;
+		} lso_v2_transmit_complete;
+		u32  value;
+	};
+};
+
 #define NDIS_VLAN_PPI_SIZE (sizeof(struct rndis_per_packet_info) + \
 		sizeof(struct ndis_pkt_8021q_info))
 
 #define NDIS_CSUM_PPI_SIZE (sizeof(struct rndis_per_packet_info) + \
 		sizeof(struct ndis_tcp_ip_checksum_info))
 
+#define NDIS_LSO_PPI_SIZE (sizeof(struct rndis_per_packet_info) + \
+		sizeof(struct ndis_tcp_lso_info))
+
 /* Format of Information buffer passed in a SetRequest for the OID */
 /* OID_GEN_RNDIS_CONFIG_PARAMETER. */
 struct rndis_config_parameter_info {
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index e8159a8..9f1f995 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -301,6 +301,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	bool isvlan;
 	struct rndis_per_packet_info *ppi;
 	struct ndis_tcp_ip_checksum_info *csum_info;
+	struct ndis_tcp_lso_info *lso_info;
 	int  hdr_offset;
 	bool is_v4;
 	bool is_tcp;
@@ -381,7 +382,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	 * GSO packet.
 	 */
 	if (skb_is_gso(skb))
-		goto do_send;
+		goto do_lso;
 
 	rndis_msg_size += NDIS_CSUM_PPI_SIZE;
 	ppi = init_ppi_data(rndis_msg, NDIS_CSUM_PPI_SIZE,
@@ -401,6 +402,35 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	} else if (is_udp) {
 		csum_info->transmit.udp_checksum = 1;
 	}
+	goto do_send;
+
+do_lso:
+	rndis_msg_size += NDIS_LSO_PPI_SIZE;
+	ppi = init_ppi_data(rndis_msg, NDIS_LSO_PPI_SIZE,
+				TCP_LARGESEND_PKTINFO);
+
+	lso_info = (struct ndis_tcp_lso_info *)((ulong)ppi +
+			ppi->ppi_offset);
+
+	lso_info->lso_v2_transmit.type = NDIS_TCP_LARGE_SEND_OFFLOAD_V2_TYPE;
+	if (is_v4) {
+		lso_info->lso_v2_transmit.ip_version =
+			NDIS_TCP_LARGE_SEND_OFFLOAD_IPV4;
+		ip_hdr(skb)->tot_len = 0;
+		ip_hdr(skb)->check = 0;
+		tcp_hdr(skb)->check =
+		~csum_tcpudp_magic(ip_hdr(skb)->saddr,
+				ip_hdr(skb)->daddr, 0, IPPROTO_TCP, 0);
+	} else {
+		lso_info->lso_v2_transmit.ip_version =
+			NDIS_TCP_LARGE_SEND_OFFLOAD_IPV6;
+		ipv6_hdr(skb)->payload_len = 0;
+		tcp_hdr(skb)->check =
+		~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+				&ipv6_hdr(skb)->daddr, 0, IPPROTO_TCP, 0);
+	}
+	lso_info->lso_v2_transmit.tcp_header_offset = hdr_offset;
+	lso_info->lso_v2_transmit.mss = skb_shinfo(skb)->gso_size;
 
 do_send:
 	/* Start filling in the page buffers with the rndis hdr */
@@ -657,10 +687,10 @@ static int netvsc_probe(struct hv_device *dev,
 
 	net->netdev_ops = &device_ops;
 
-	/* TODO: Add GSO and Checksum offload */
-	net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM;
+	net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM |
+				NETIF_F_TSO;
 	net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM |
-			NETIF_F_IP_CSUM;
+			NETIF_F_IP_CSUM | NETIF_F_TSO;
 
 	SET_ETHTOOL_OPS(net, &ethtool_ops);
 	SET_NETDEV_DEV(net, &dev->device);
-- 
1.7.4.1

Re: [PATCH 1/6] Drivers: net: hyperv: Enable scatter gather I/O

[David Miller]

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Thu,  6 Mar 2014 03:13:05 -0800

> @@ -140,21 +140,125 @@ static void netvsc_xmit_completion(void *context)
>  		dev_kfree_skb_any(skb);
>  }
>  
> +static u32 fill_pg_buf(struct page *page, u32 offset, u32 len,
> +			struct hv_page_buffer *pb)
> +{
> +	int j = 0;
> +	/*
> +	 * Deal with compund pages by ignoring unused part
> +	 * of the page.
> +	 */
> +	page += offset  >> PAGE_SHIFT;
> +	offset &= ~PAGE_MASK;

Please put an empty line between local variable declarations and
the code.

Please format comments:

	/* Like
	 * this.
	 */

> +	while (len > 0) {
> +		unsigned long bytes;
> +		bytes = PAGE_SIZE - offset;

Empty line between local variable declarations and code.

> +	/*
> +	 * The packet is laid out thus:
> +	 * 1. hdr
> +	 * 2. skb linear data
> +	 * 3. skb fragment data
> +	 */

Fix comment formatting as explained above.

> +	if (hdr != NULL) {
> +		slots_used += fill_pg_buf(virt_to_page(hdr),
> +					offset_in_page(hdr),
> +					len, &pb[slots_used]);
> +	}

Single statment basic blocks do not need surrounding braces.

> +	/*
> +	 * We will atmost need two pages to describe the rndis
> +	 * header. We can only transmit MAX_PAGE_BUFFER_COUNT number
> +	 * of pages in a single packet.
> +	 */

Comment formatting.

Re: [PATCH 2/6] Drivers: net: hyperv: Cleanup the send path

[David Miller]

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Thu,  6 Mar 2014 03:13:06 -0800

> In preparation for enabling offloads, cleanup the send path.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/net/hyperv/hyperv_net.h   |    7 +---
>  drivers/net/hyperv/netvsc_drv.c   |   87 +++++++++++++++++++++++++++++-------
>  drivers/net/hyperv/rndis_filter.c |   66 ----------------------------
>  3 files changed, 71 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index 39fc230..694bf7c 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -73,7 +73,7 @@ struct hv_netvsc_packet {
>  	} completion;
>  
>  	/* This points to the memory after page_buf */
> -	void *extension;
> +	struct rndis_message *rndis_msg;
>  
>  	u32 total_data_buflen;
>  	/* Points to the send/receive buffer where the ethernet frame is */
> @@ -126,11 +126,6 @@ void rndis_filter_device_remove(struct hv_device *dev);
>  int rndis_filter_receive(struct hv_device *dev,
>  			struct hv_netvsc_packet *pkt);
>  
> -
> -
> -int rndis_filter_send(struct hv_device *dev,
> -			struct hv_netvsc_packet *pkt);
> -
>  int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter);
>  int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac);
>  
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index afa9c53..e812529 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -128,6 +128,28 @@ static int netvsc_close(struct net_device *net)
>  	return ret;
>  }
>  
> +
> +static void *init_ppi_data(struct rndis_message *msg, u32 ppi_size,

One empty line between functions is sufficient.

> @@ -173,7 +195,7 @@ static u32 fill_pg_buf(struct page *page, u32 offset, u32 len,
>  	return j + 1;
>  }
>  
> -static void init_page_array(void *hdr, u32 len, struct sk_buff *skb,
> +static u32 init_page_array(void *hdr, u32 len, struct sk_buff *skb,
>  				struct hv_page_buffer *pb)

When you adjust where the openning parenthesis occurs in a function
definition or call, you must adjust the indentation of arguments that
occur on the following lines.

In particular, such arguments must start at exactly the first column
after the openning parenthesis.  You must use the appropriate number
of TAB and space characters necessary to achieve this.

Re: [PATCH 3/6] Drivers: net: hyperv: Enable offloads on the host

[David Miller]

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Thu,  6 Mar 2014 03:13:07 -0800

> +		/*
> +		 * can't put_rndis_request, since we may still receive a
> +		 * send-completion.
> +		 */

Please fix the formatting of this comment.

> +	/*
> +	 * Turn on the offloads; the host supports all of the relevant
> +	 * offloads.
> +	 */

Likewise.

> +	/*
> +	 * A value of zero means "no change"; now turn on what we
> +	 * want.
> +	 */

Likewise.

Re: [PATCH 4/6] Drivers: net: hyperv: Enable receive side IP checksum offload

[David Miller]

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Thu,  6 Mar 2014 03:13:08 -0800

> +		/*
> +		 * We only look at the IP checksum here.
> +		 * Should we be dropping the packet if checksum
> +		 * failed? How do we deal with other checksums - TCP/UDP?
> +		 */

Please fix the formatting of this cmment.

Re: [PATCH 5/6] Drivers: net: hyperv: Enable send side checksum offload

[David Miller]

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Thu,  6 Mar 2014 03:13:09 -0800

> +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4,
> +				bool *is_tcp, bool *is_udp, u32 *trans_off)
> +{

Returning so many values like this is awkward, at best.

Why not return a well defined bitmask:

#define TRANSPORT_INFO_SUCCESS	0x00000001
#define TRANSPORT_INFO_TCP	0x00000002
#define TRANSPORT_INFO_UDP	0x00000004

Then the only value you have to return by reference is trans_off.

Re: [PATCH 6/6] Drivers: net: hyperv: Enable large send offload

[David Miller]

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Thu,  6 Mar 2014 03:13:10 -0800

> +	ppi = init_ppi_data(rndis_msg, NDIS_LSO_PPI_SIZE,
> +				TCP_LARGESEND_PKTINFO);

This is not indented properly, "TCP_LARGESEND_PKTINFO);" should start at the
first column after the openning parenthesis of "init_ppi_data(".

> +		~csum_tcpudp_magic(ip_hdr(skb)->saddr,
> +				ip_hdr(skb)->daddr, 0, IPPROTO_TCP, 0);

Likewise.

Re: [PATCH 3/6] Drivers: net: hyperv: Enable offloads on the host

[Dan Carpenter]

On Thu, Mar 06, 2014 at 03:13:07AM -0800, K. Y. Srinivasan wrote:
> +	offload_params = (struct ndis_offload_params *)((ulong)set +
> +				set->info_buf_offset);

It's a bit simpler to do pointer math like this:

	offload_params = (void *)set + set->info_buf_offset;

regards,
dan carpenter

RE: [PATCH 5/6] Drivers: net: hyperv: Enable send side checksum offload

[KY Srinivasan]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, March 7, 2014 1:04 AM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 5/6] Drivers: net: hyperv: Enable send side checksum
> offload
> 
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> Date: Thu,  6 Mar 2014 03:13:09 -0800
> 
> > +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4,
> > +				bool *is_tcp, bool *is_udp, u32 *trans_off) {
> 
> Returning so many values like this is awkward, at best.
> 
> Why not return a well defined bitmask:
> 
> #define TRANSPORT_INFO_SUCCESS	0x00000001
> #define TRANSPORT_INFO_TCP	0x00000002
> #define TRANSPORT_INFO_UDP	0x00000004
> 
> Then the only value you have to return by reference is trans_off.

I also need information on the IP version as well. If it is ok with you, I will use the return
value to get information on the network protocol and return information on transport via
reference - protocol and the offset.
 
Regards,

K. Y

Re: [PATCH 5/6] Drivers: net: hyperv: Enable send side checksum offload

[David Miller]

From: KY Srinivasan <kys@microsoft.com>
Date: Thu, 6 Mar 2014 20:29:13 +0000

> 
> 
>> -----Original Message-----
>> From: David Miller [mailto:davem@davemloft.net]
>> Sent: Friday, March 7, 2014 1:04 AM
>> To: KY Srinivasan
>> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> jasowang@redhat.com
>> Subject: Re: [PATCH 5/6] Drivers: net: hyperv: Enable send side checksum
>> offload
>> 
>> From: "K. Y. Srinivasan" <kys@microsoft.com>
>> Date: Thu,  6 Mar 2014 03:13:09 -0800
>> 
>> > +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4,
>> > +				bool *is_tcp, bool *is_udp, u32 *trans_off) {
>> 
>> Returning so many values like this is awkward, at best.
>> 
>> Why not return a well defined bitmask:
>> 
>> #define TRANSPORT_INFO_SUCCESS	0x00000001
>> #define TRANSPORT_INFO_TCP	0x00000002
>> #define TRANSPORT_INFO_UDP	0x00000004
>> 
>> Then the only value you have to return by reference is trans_off.
> 
> I also need information on the IP version as well. If it is ok with you, I will use the return
> value to get information on the network protocol and return information on transport via
> reference - protocol and the offset.

Just add "TRANSPORT_INFO_IPv4" as another bitmask in the return value.

RE: [PATCH 5/6] Drivers: net: hyperv: Enable send side checksum offload

[KY Srinivasan]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, March 7, 2014 2:19 AM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 5/6] Drivers: net: hyperv: Enable send side checksum
> offload
> 
> From: KY Srinivasan <kys@microsoft.com>
> Date: Thu, 6 Mar 2014 20:29:13 +0000
> 
> >
> >
> >> -----Original Message-----
> >> From: David Miller [mailto:davem@davemloft.net]
> >> Sent: Friday, March 7, 2014 1:04 AM
> >> To: KY Srinivasan
> >> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> >> jasowang@redhat.com
> >> Subject: Re: [PATCH 5/6] Drivers: net: hyperv: Enable send side
> >> checksum offload
> >>
> >> From: "K. Y. Srinivasan" <kys@microsoft.com>
> >> Date: Thu,  6 Mar 2014 03:13:09 -0800
> >>
> >> > +bool get_net_transport_info(struct sk_buff *skb, bool *is_v4,
> >> > +				bool *is_tcp, bool *is_udp, u32 *trans_off) {
> >>
> >> Returning so many values like this is awkward, at best.
> >>
> >> Why not return a well defined bitmask:
> >>
> >> #define TRANSPORT_INFO_SUCCESS	0x00000001
> >> #define TRANSPORT_INFO_TCP	0x00000002
> >> #define TRANSPORT_INFO_UDP	0x00000004
> >>
> >> Then the only value you have to return by reference is trans_off.
> >
> > I also need information on the IP version as well. If it is ok with
> > you, I will use the return value to get information on the network
> > protocol and return information on transport via reference - protocol and
> the offset.
> 
> Just add "TRANSPORT_INFO_IPv4" as another bitmask in the return value.

The network information is independent of the transport information - I can partition the
return value to encode all the possible network and transport combination and subsequently
parse the network and transport information. I will go ahead and code it up this way.

Regards,

K. Y

 

[PATCH] checkpatch: net and drivers/net: Warn on missing blank line after variable declaration

[Joe Perches]

Networking prefers this style, so warn when it's not used.

Networking uses:

void foo(int bar)
{
	int baz;

	code...
}

not

void foo(int bar)
{
	int baz;
	code...
}

There are a limited number of false positives when using
macros to declare variables like:

WARNING: networking uses a blank line after declarations
#330: FILE: net/ipv4/inet_hashtables.c:330:
+	int dif = sk->sk_bound_dev_if;
+	INET_ADDR_COOKIE(acookie, saddr, daddr)

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3d3ef2f..a6e3048 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2248,6 +2248,21 @@ sub process {
 			     "networking block comments put the trailing */ on a separate line\n" . $herecurr);
 		}
 
+# check for missing blank lines after declarations
+		if ($realfile =~ m@^(drivers/net/|net/)@ &&
+		    $prevline =~ /^\+\s+$Declare\s+$Ident/ &&
+		    !($prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ ||
+		      $prevline =~ /(?:\{\s*|\\)$/) &&		#extended lines
+		    $sline =~ /^\+\s+/ &&			#Not at char 1
+		    !($sline =~ /^\+\s+$Declare/ ||
+		      $sline =~ /^\+\s+$Ident\s+$Ident/ ||	#eg: typedef foo
+		      $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ ||
+		      $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(])/ ||
+		      $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) {
+			WARN("SPACING",
+			     "networking uses a blank line after declarations\n" . $hereprev);
+		}
+
 # check for spaces at the beginning of a line.
 # Exceptions:
 #  1) within comments

Re: [PATCH] checkpatch: net and drivers/net: Warn on missing blank line after variable declaration

[Andrew Morton]

On Thu, 06 Mar 2014 15:28:40 -0800 Joe Perches <joe@perches.com> wrote:

> Networking prefers this style, so warn when it's not used.
> 
> Networking uses:
> 
> void foo(int bar)
> {
> 	int baz;
> 
> 	code...
> }
> 
> not
> 
> void foo(int bar)
> {
> 	int baz;
> 	code...
> }
> 
> There are a limited number of false positives when using
> macros to declare variables like:
> 
> WARNING: networking uses a blank line after declarations
> #330: FILE: net/ipv4/inet_hashtables.c:330:
> +	int dif = sk->sk_bound_dev_if;
> +	INET_ADDR_COOKIE(acookie, saddr, daddr)

um wait wut wot.

*All* kernel code uses blank line between end-of-locals and
start-of-code.  Or if it doesn't it should, thwap.

Why are we special-casing net/?

Re: [PATCH] checkpatch: net and drivers/net: Warn on missing blank line after variable declaration

[Joe Perches]

On Thu, 2014-03-06 at 15:35 -0800, Andrew Morton wrote:
> On Thu, 06 Mar 2014 15:28:40 -0800 Joe Perches <joe@perches.com> wrote:
> > Networking prefers this style, so warn when it's not used.
> > void foo(int bar)
> > {
> > 	int baz;
> > 
> > 	code...
> > }
> > 
> > not
> > 
> > void foo(int bar)
> > {
> > 	int baz;
> > 	code...
> > }
> > 
> > There are a limited number of false positives when using
> > macros to declare variables like:
> > 
> > WARNING: networking uses a blank line after declarations
> > #330: FILE: net/ipv4/inet_hashtables.c:330:
> > +	int dif = sk->sk_bound_dev_if;
> > +	INET_ADDR_COOKIE(acookie, saddr, daddr)
> 
> um wait wut wot.
> 
> *All* kernel code uses blank line between end-of-locals and
> start-of-code.  Or if it doesn't it should, thwap.
> Why are we special-casing net/?

It's easy enough to remove the path test, but it's
not in CodingStyle and David seems to be the one
that makes the effort to correct people about it.

I don't care one way or another.

I'm just trying to get fewer rejections for people
that use checkpatch.

cheers, Joe

Re: [PATCH] checkpatch: net and drivers/net: Warn on missing blank line after variable declaration

[Andrew Morton]

On Thu, 06 Mar 2014 15:42:30 -0800 Joe Perches <joe@perches.com> wrote:

> On Thu, 2014-03-06 at 15:35 -0800, Andrew Morton wrote:
> > On Thu, 06 Mar 2014 15:28:40 -0800 Joe Perches <joe@perches.com> wrote:
> > > Networking prefers this style, so warn when it's not used.
> > > void foo(int bar)
> > > {
> > > 	int baz;
> > > 
> > > 	code...
> > > }
> > > 
> > > not
> > > 
> > > void foo(int bar)
> > > {
> > > 	int baz;
> > > 	code...
> > > }
> > > 
> > > There are a limited number of false positives when using
> > > macros to declare variables like:
> > > 
> > > WARNING: networking uses a blank line after declarations
> > > #330: FILE: net/ipv4/inet_hashtables.c:330:
> > > +	int dif = sk->sk_bound_dev_if;
> > > +	INET_ADDR_COOKIE(acookie, saddr, daddr)
> > 
> > um wait wut wot.
> > 
> > *All* kernel code uses blank line between end-of-locals and
> > start-of-code.  Or if it doesn't it should, thwap.
> > Why are we special-casing net/?
> 
> It's easy enough to remove the path test, but it's
> not in CodingStyle and David seems to be the one
> that makes the effort to correct people about it.
> 
> I don't care one way or another.
> 
> I'm just trying to get fewer rejections for people
> that use checkpatch.

mutter.

OK, let's do this for now.  Could you please cook up a followon patch
which makes this kernel-wide?  I'll play with that for a while then I'll
decide how much I feel like irritating people.

[PATCH] checkpatch: Always warn on missing blank line after variable declaration block

[Joe Perches]

Make the test system wide, modify the message too.

Signed-off-by: Joe Perches <joe@perches.com>
---
> OK, let's do this for now.  Could you please cook up a followon patch
> which makes this kernel-wide?  I'll play with that for a while then I'll
> decide how much I feel like irritating people.

 scripts/checkpatch.pl | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a6e3048..37a94f1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2249,8 +2249,7 @@ sub process {
 		}
 
 # check for missing blank lines after declarations
-		if ($realfile =~ m@^(drivers/net/|net/)@ &&
-		    $prevline =~ /^\+\s+$Declare\s+$Ident/ &&
+		if ($prevline =~ /^\+\s+$Declare\s+$Ident/ &&
 		    !($prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ ||
 		      $prevline =~ /(?:\{\s*|\\)$/) &&		#extended lines
 		    $sline =~ /^\+\s+/ &&			#Not at char 1
@@ -2260,7 +2259,7 @@ sub process {
 		      $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(])/ ||
 		      $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) {
 			WARN("SPACING",
-			     "networking uses a blank line after declarations\n" . $hereprev);
+			     "Missing a blank line after declarations\n" . $hereprev);
 		}
 
 # check for spaces at the beginning of a line.

Re: [PATCH] checkpatch: net and drivers/net: Warn on missing blank line after variable declaration

[Dan Carpenter]

How many warnings does this generate does this generate when you run it
across the whole tree?

regards,
dan carpenter

Re: [PATCH] checkpatch: net and drivers/net: Warn on missing blank line after variable declaration

[Joe Perches]

On Fri, 2014-03-07 at 10:54 +0300, Dan Carpenter wrote:
> How many warnings does this generate does this generate when you run it
> across the whole tree?

A lot.

Check back with me after the week or so
it'll take to run on this little netboook.

Try this with both patches applied if you want
to know yourself...

$ git ls-files | grep "\.[ch]$"| \
  xargs ./scripts/checkpatch.pl -f --strict --types=spacing --emacs --terse | \
  grep "Missing a blank"

Re: [PATCH 5/6] Drivers: net: hyperv: Enable send side checksum offload

[Ben Hutchings]

[-- Attachment #1.1: Type: text/plain, Size: 745 bytes --]

On Thu, 2014-03-06 at 03:13 -0800, K. Y. Srinivasan wrote:
[...]
> @@ -593,8 +658,9 @@ static int netvsc_probe(struct hv_device *dev,
>  	net->netdev_ops = &device_ops;
>  
>  	/* TODO: Add GSO and Checksum offload */
> -	net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG;
> -	net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM;
> +	net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM;
> +	net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM |
> +			NETIF_F_IP_CSUM;

Missing NETIF_F_IPV6_CSUM?

Ben.

>  	SET_ETHTOOL_OPS(net, &ethtool_ops);
>  	SET_NETDEV_DEV(net, &dev->device);

-- 
Ben Hutchings
I say we take off; nuke the site from orbit.  It's the only way to be sure.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Treewide frequency of various checkpatch messages

[Joe Perches]

On Fri, 2014-03-07 at 01:30 -0800, Joe Perches wrote:
> On Fri, 2014-03-07 at 10:54 +0300, Dan Carpenter wrote:
 (a question about a new message warning of a missing
  blank line between variable declaration blocks and
  code in a function)
> > How many warnings does this generate does this generate when you run it
> > across the whole tree?
> A lot.

Turns out it's 20,210 and it's the 14th
most common checkpatch message type.

    14    20210  WARNING:SPACING: Missing a blank line after declarations

> Check back with me after the week or so
> it'll take to run on this little netboook.

I ran a variant of this over the weekend
over next-20140306.  It just finished.

$ git ls-files | grep "\.[ch]$"| \
  while read file ; do \
    ./scripts/checkpatch.pl -f --strict --show-types --no-summary --emacs --terse $file ; \
  done | \
  tee -a checkpatch.all

Here are the top 100 types of messages:

(for    2377922 total LOC)

     1   241524  WARNING:LONG_LINE: line over 80 characters
     2   238492  ERROR:SPACING: space required after that ',' (ctx:VxV)
     3   152039  WARNING:LEADING_SPACE: please, no spaces at the start of a line
     4   117364  CHECK:CAMELCASE: Avoid CamelCase
     5    67761  CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
     6    53673  ERROR:CODE_INDENT: code indent should use tabs where possible
     7    30981  ERROR:TRAILING_WHITESPACE: trailing whitespace
     8    30443  ERROR:C99_COMMENTS: do not use C99 // comments
     9    26928  WARNING:SPACE_BEFORE_TAB: please, no space before tabs
    10    25546  CHECK:AVOID_EXTERNS: extern prototypes should be avoided in .h files
    11    25384  WARNING:SPACING: space prohibited between function name and open parenthesis '('
    12    25209  WARNING:SPLIT_STRING: quoted string split across lines
    13    21410  WARNING:NETWORKING_BLOCK_COMMENT_STYLE: networking block comments don't use an empty /* line, use /* Comment...
    14    20210  WARNING:SPACING: Missing a blank line after declarations
    15    19881  CHECK:SPACING: No space is necessary after a cast
    16    18045  ERROR:SPACING: space prohibited after that open parenthesis '('
    17    17009  ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parenthesis
    18    16674  ERROR:SPACING: space prohibited before that close parenthesis ')'
    19    14364  ERROR:SPACING: space required before the open parenthesis '('
    20    13331  ERROR:TRAILING_STATEMENTS: trailing statements should be on next line
    21    12619  CHECK:BRACES: braces {} should be used on all arms of this statement
    22    12605  CHECK:FSF_MAILING_ADDRESS: Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The
    23    11194  ERROR:SPACING: spaces prohibited around that ':' (ctx:WxW)
    24    10192  ERROR:OPEN_BRACE: that open brace { should be on the previous line
    25     9941  WARNING:PREFER_PR_LEVEL: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(...  to printk(KERN_ERR ...
    26     9355  CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
    27     9215  WARNING:NEW_TYPEDEFS: do not add new typedefs
    28     8733  ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar"
    29     8129  WARNING:PRINTK_WITHOUT_KERN_LEVEL: printk() should include KERN_ facility level
    30     7676  WARNING:BRACES: braces {} are not necessary for single statement blocks
    31     7339  CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the previous line
    32     7247  ERROR:ASSIGN_IN_IF: do not use assignment in if condition
    33     6918  WARNING:NETWORKING_BLOCK_COMMENT_STYLE: networking block comments put the trailing */ on a separate line
    34     6750  ERROR:SPACING: spaces required around that '=' (ctx:VxV)
    35     6446  WARNING:SPACING: space prohibited before semicolon
    36     6289  WARNING:PREFER_PR_LEVEL: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(...  to printk(KERN_INFO ...
    37     6276  CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
    38     6204  WARNING:VOLATILE: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
    39     5921  CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
    40     5262  WARNING:QUOTED_WHITESPACE_BEFORE_NEWLINE: unnecessary whitespace before a quoted newline
    41     5050  WARNING:NETWORKING_BLOCK_COMMENT_STYLE: networking block comments start with * on subsequent lines
    42     4740  WARNING:PREFER_PR_LEVEL: Prefer [subsystem eg: netdev]_warn([subsystem]dev, ... then dev_warn(dev, ... then pr_warn(...  to printk(KERN_WARNING ...
    43     4409  WARNING:PREFER_PR_LEVEL: Prefer [subsystem eg: netdev]_dbg([subsystem]dev, ... then dev_dbg(dev, ... then pr_debug(...  to printk(KERN_DEBUG ...
    44     4248  ERROR:SPACING: space required after that close brace '}'
    45     4237  ERROR:POINTER_LOCATION: "foo* bar" should be "foo *bar"
    46     4224  WARNING:EXPORT_SYMBOL: EXPORT_SYMBOL(foo); should immediately follow its function/variable
    47     4164  ERROR:RETURN_PARENTHESES: return is not a function, parentheses are not required
    48     3734  CHECK:ALLOC_SIZEOF_STRUCT: Prefer
    49     3651  CHECK:USLEEP_RANGE: usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt
    50     3597  WARNING:AVOID_EXTERNS: externs should be avoided in .c files
    51     3515  WARNING:SPACING: Unnecessary space before function pointer arguments
    52     3241  ERROR:ELSE_AFTER_BRACE: else should follow close brace '}'
    53     2995  WARNING:MEMORY_BARRIER: memory barrier without comment
    54     2713  ERROR:SPACING: space prohibited before that close square bracket ']'
    55     2337  ERROR:POINTER_LOCATION: "(foo*)" should be "(foo *)"
    56     2329  CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment
    57     2298  WARNING:PREFER_PACKED: __packed is preferred over __attribute__((packed))
    58     2180  ERROR:OPEN_BRACE: open brace '{' following struct go on the same line
    59     2032  CHECK:REDUNDANT_CODE: if this code is redundant consider removing it
    60     2017  ERROR:SPACING: space prohibited after that open square bracket '['
    61     1945  WARNING:BRACES: braces {} are not necessary for any arm of this statement
    62     1898  WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt
    63     1884  WARNING:MISSING_BREAK: Possible switch case/default not preceeded by break or fallthrough comment
    64     1869  WARNING:CONSTANT_CONVERSION: __constant_cpu_to_le32 should be cpu_to_le32
    65     1855  WARNING:PREFER_ETHER_ADDR_COPY: Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)
    66     1800  WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 12)
    67     1779  ERROR:SPACING: space required after that ',' (ctx:VxO)
    68     1773  ERROR:BRACKET_SPACE: space prohibited before open square bracket '['
    69     1664  WARNING:INDENTED_LABEL: labels should not be indented
    70     1620  ERROR:SPACING: need consistent spacing around '%' (ctx:WxV)
    71     1594  CHECK:SPACING: spaces required around that ':' (ctx:VxV)
    72     1519  ERROR:SPACING: space required before the open brace '{'
    73     1414  WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably be static const char * const
    74     1392  ERROR:SPACING: space required after that ';' (ctx:VxV)
    75     1376  WARNING:PREFER_SEQ_PUTS: Prefer seq_puts to seq_printf
    76     1363  WARNING:DEEP_INDENTATION: Too many leading tabs - consider code refactoring
    77     1328  CHECK:UNCOMMENTED_DEFINITION: struct mutex definition without comment
    78     1316  ERROR:INITIALISED_STATIC: do not initialise statics to 0 or NULL
    79     1287  WARNING:LINE_CONTINUATIONS: Avoid unnecessary line continuations
    80     1239  ERROR:SPACING: spaces required around that '<' (ctx:VxV)
    81     1181  ERROR:SWITCH_CASE_INDENT_LEVEL: switch and case should be at the same indent
    82     1129  ERROR:MULTISTATEMENT_MACRO_USE_DO_WHILE: Macros with multiple statements should be enclosed in a do - while loop
    83     1036  WARNING:PREFER_PR_LEVEL: Prefer pr_warn(... to pr_warning(...
    84      942  ERROR:SPACING: spaces required around that '==' (ctx:VxV)
    85      941  WARNING:INCLUDE_LINUX: Use #include <linux/io.h> instead of <asm/io.h>
    86      906  WARNING:CONSIDER_KSTRTO: simple_strtoul is obsolete, use kstrtoul instead
    87      836  ERROR:SPACING: spaces prohibited around that ':' (ctx:WxV)
    88      805  WARNING:PREFER_PR_LEVEL: Prefer [subsystem eg: netdev]_notice([subsystem]dev, ... then dev_notice(dev, ... then pr_notice(...  to printk(KERN_NOTICE ..
    89      803  ERROR:SPACING: space prohibited after that '!' (ctx:BxW)
    90      787  ERROR:SPACING: space required before that '-' (ctx:OxV)
    91      776  ERROR:SPACING: spaces required around that ':' (ctx:VxW)
    92      765  WARNING:INCLUDE_LINUX: Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
    93      759  CHECK:INVALID_UTF8: Invalid UTF-8, patch and commit message should be encoded in UTF-8
    94      754  ERROR:SPACING: space required before that '&' (ctx:OxV)
    95      723  WARNING:SINGLE_STATEMENT_DO_WHILE_MACRO: Single statement macros should not use a do {} while (0) loop
    96      710  CHECK:ARCH_INCLUDE_LINUX: Consider using #include <linux/io.h> instead of <asm/io.h>
    97      707  ERROR:SPACING: spaces required around that '=' (ctx:VxW)
    98      691  WARNING:CVS_KEYWORD: CVS style keyword markers, these will _not_ be updated
    99      644  WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (2, 4)
   100      641  ERROR:OPEN_BRACE: open brace '{' following function declarations go on the next line

Re: Treewide frequency of various checkpatch messages

[Greg KH]

On Mon, Mar 10, 2014 at 09:02:26AM -0700, Joe Perches wrote:
> On Fri, 2014-03-07 at 01:30 -0800, Joe Perches wrote:
> > On Fri, 2014-03-07 at 10:54 +0300, Dan Carpenter wrote:
>  (a question about a new message warning of a missing
>   blank line between variable declaration blocks and
>   code in a function)
> > > How many warnings does this generate does this generate when you run it
> > > across the whole tree?
> > A lot.
> 
> Turns out it's 20,210 and it's the 14th
> most common checkpatch message type.
> 
>     14    20210  WARNING:SPACING: Missing a blank line after declarations

I think it's still worthwhile to clean up.

thanks,

greg k-h

Re: Treewide frequency of various checkpatch messages

[Joe Perches]

On Mon, 2014-03-10 at 09:50 -0700, Greg KH wrote:
> On Mon, Mar 10, 2014 at 09:02:26AM -0700, Joe Perches wrote:
> > On Fri, 2014-03-07 at 01:30 -0800, Joe Perches wrote:
> > > On Fri, 2014-03-07 at 10:54 +0300, Dan Carpenter wrote:
> >  (a question about a new message warning of a missing
> >   blank line between variable declaration blocks and
> >   code in a function)
> > > > How many warnings does this generate does this generate when you run it
> > > > across the whole tree?
> > > A lot.
> > 
> > Turns out it's 20,210 and it's the 14th
> > most common checkpatch message type.
> > 
> >     14    20210  WARNING:SPACING: Missing a blank line after declarations
> 
> I think it's still worthwhile to clean up.

Maybe.

Luckily, <smile> I don't have to deal with the
patches that would be generated by this message.

Some people are going to view patches for this as
useless noise.

Couple of things:

It's kind of interesting how the messages vary by
subsystem.  Let me know if you want any breakdowns.

And there are a small number of false positives for
this "Missing a blank line" test with declarations
like:

	typedef *foo;
	DECLARE_BITMAP(foo);
	__DECL_REG(foo);
	LIST_HEAD(foo);

So there could be a minor improvement to the test.

I looked at some of the results using:

This sort of match stands out a bit:

---> arch/tile/lib/spinlock_32.c:68:
{
        u32 iterations = 0;
        while (arch_spin_is_locked(lock))
                delay_backoff(iterations++);
}

Instances like this may be fine, but adding blank
lines to very short functions with a single
declaration just adds to the overall line count.

I've no strong opinion of the need to write code
like:

{
        u32 iterations = 0;

        while (arch_spin_is_locked(lock))
                delay_backoff(iterations++);
}

Re: Treewide frequency of various checkpatch messages

[Greg KH]

On Mon, Mar 10, 2014 at 11:48:50AM -0700, Joe Perches wrote:
> On Mon, 2014-03-10 at 09:50 -0700, Greg KH wrote:
> > On Mon, Mar 10, 2014 at 09:02:26AM -0700, Joe Perches wrote:
> > > On Fri, 2014-03-07 at 01:30 -0800, Joe Perches wrote:
> > > > On Fri, 2014-03-07 at 10:54 +0300, Dan Carpenter wrote:
> > >  (a question about a new message warning of a missing
> > >   blank line between variable declaration blocks and
> > >   code in a function)
> > > > > How many warnings does this generate does this generate when you run it
> > > > > across the whole tree?
> > > > A lot.
> > > 
> > > Turns out it's 20,210 and it's the 14th
> > > most common checkpatch message type.
> > > 
> > >     14    20210  WARNING:SPACING: Missing a blank line after declarations
> > 
> > I think it's still worthwhile to clean up.
> 
> Maybe.
> 
> Luckily, <smile> I don't have to deal with the
> patches that would be generated by this message.
> 
> Some people are going to view patches for this as
> useless noise.

That's true for all checkpatch cleanups :)

> Couple of things:
> 
> It's kind of interesting how the messages vary by
> subsystem.  Let me know if you want any breakdowns.
> 
> And there are a small number of false positives for
> this "Missing a blank line" test with declarations
> like:
> 
> 	typedef *foo;
> 	DECLARE_BITMAP(foo);
> 	__DECL_REG(foo);
> 	LIST_HEAD(foo);
> 
> So there could be a minor improvement to the test.
> 
> I looked at some of the results using:
> 
> This sort of match stands out a bit:
> 
> ---> arch/tile/lib/spinlock_32.c:68:
> {
>         u32 iterations = 0;
>         while (arch_spin_is_locked(lock))
>                 delay_backoff(iterations++);
> }
> 
> Instances like this may be fine, but adding blank
> lines to very short functions with a single
> declaration just adds to the overall line count.
> 
> I've no strong opinion of the need to write code
> like:
> 
> {
>         u32 iterations = 0;
> 
>         while (arch_spin_is_locked(lock))
>                 delay_backoff(iterations++);
> }

I wonder if there's a way to "count" the size of the function, and only
complain if it's longer than 4-5 lines long?

thanks,

greg k-h

Re: Treewide frequency of various checkpatch messages

[Joe Perches]

On Mon, 2014-03-10 at 12:33 -0700, Greg KH wrote:
> On Mon, Mar 10, 2014 at 11:48:50AM -0700, Joe Perches wrote:
[]
> > I've no strong opinion of the need to write code
> > like:
> > 
> > {
> >         u32 iterations = 0;
> > 
> >         while (arch_spin_is_locked(lock))
> >                 delay_backoff(iterations++);
> > }
> 
> I wonder if there's a way to "count" the size of the function, and only
> complain if it's longer than 4-5 lines long?

No, there's isn't really.
checkpatch works on patch hunks.
The function scope isn't necessarily visible.