Linux-HyperV List
 help / color / mirror / Atom feed
* [PATCH net-next 1/3] net: core: add helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-17 21:40 UTC (permalink / raw)
  To: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc.
  Cc: netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <76cd6cfc-f4f3-ece7-203a-0266b7f02a12@gmail.com>

Several network drivers for chips that support TSO6 share the same code
for preparing the TCP header. A difference is that some reset the
payload_len whilst others don't do this. Let's factor out this common
code to a new helper.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 include/net/ip6_checksum.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
index 7bec95df4..ef0130023 100644
--- a/include/net/ip6_checksum.h
+++ b/include/net/ip6_checksum.h
@@ -76,6 +76,18 @@ static inline void __tcp_v6_send_check(struct sk_buff *skb,
 	}
 }
 
+static inline void tcp_v6_gso_csum_prep(struct sk_buff *skb,
+					bool clear_payload_len)
+{
+	struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+	struct tcphdr *th = tcp_hdr(skb);
+
+	if (clear_payload_len)
+		ipv6h->payload_len = 0;
+
+	th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 static inline void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
 {
-- 
2.25.0



^ permalink raw reply related

* [PATCH net-next 2/3] r8169: use new helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-17 21:40 UTC (permalink / raw)
  To: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc.
  Cc: netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <76cd6cfc-f4f3-ece7-203a-0266b7f02a12@gmail.com>

Simplify the code by using new helper tcp_v6_gso_csum_prep.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 26 ++---------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 5a9143b50..75ba10069 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4108,29 +4108,6 @@ static bool rtl_test_hw_pad_bug(struct rtl8169_private *tp, struct sk_buff *skb)
 	return skb->len < ETH_ZLEN && tp->mac_version == RTL_GIGA_MAC_VER_34;
 }
 
-/* msdn_giant_send_check()
- * According to the document of microsoft, the TCP Pseudo Header excludes the
- * packet length for IPv6 TCP large packets.
- */
-static int msdn_giant_send_check(struct sk_buff *skb)
-{
-	const struct ipv6hdr *ipv6h;
-	struct tcphdr *th;
-	int ret;
-
-	ret = skb_cow_head(skb, 0);
-	if (ret)
-		return ret;
-
-	ipv6h = ipv6_hdr(skb);
-	th = tcp_hdr(skb);
-
-	th->check = 0;
-	th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
-
-	return ret;
-}
-
 static void rtl8169_tso_csum_v1(struct sk_buff *skb, u32 *opts)
 {
 	u32 mss = skb_shinfo(skb)->gso_size;
@@ -4163,9 +4140,10 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp,
 			break;
 
 		case htons(ETH_P_IPV6):
-			if (msdn_giant_send_check(skb))
+			if (skb_cow_head(skb, 0))
 				return false;
 
+			tcp_v6_gso_csum_prep(skb, false);
 			opts[0] |= TD1_GTSENV6;
 			break;
 
-- 
2.25.0



^ permalink raw reply related

* [PATCH net-next 3/3] net: use new helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-17 21:42 UTC (permalink / raw)
  To: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc.
  Cc: netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <76cd6cfc-f4f3-ece7-203a-0266b7f02a12@gmail.com>

Use new helper tcp_v6_gso_csum_prep in additional network drivers.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/atheros/alx/main.c       |  5 +---
 .../net/ethernet/atheros/atl1c/atl1c_main.c   |  6 ++---
 drivers/net/ethernet/brocade/bna/bnad.c       |  7 +----
 drivers/net/ethernet/cisco/enic/enic_main.c   |  3 +--
 drivers/net/ethernet/intel/e1000/e1000_main.c |  6 +----
 drivers/net/ethernet/intel/e1000e/netdev.c    |  5 +---
 drivers/net/ethernet/jme.c                    |  7 +----
 .../net/ethernet/pensando/ionic/ionic_txrx.c  |  5 +---
 drivers/net/ethernet/qualcomm/emac/emac-mac.c |  7 ++---
 drivers/net/ethernet/socionext/netsec.c       |  6 +----
 drivers/net/hyperv/netvsc_drv.c               |  5 +---
 drivers/net/usb/r8152.c                       | 26 ++-----------------
 drivers/net/vmxnet3/vmxnet3_drv.c             |  5 +---
 13 files changed, 16 insertions(+), 77 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 1dcbc486e..3e0215887 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1416,10 +1416,7 @@ static int alx_tso(struct sk_buff *skb, struct alx_txd *first)
 							 0, IPPROTO_TCP, 0);
 		first->word1 |= 1 << TPD_IPV4_SHIFT;
 	} else if (skb_is_gso_v6(skb)) {
-		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);
+		tcp_v6_gso_csum_prep(skb, true);
 		/* LSOv2: the first TPD only provides the packet length */
 		first->adrl.l.pkt_len = skb->len;
 		first->word1 |= 1 << TPD_LSO_V2_SHIFT;
diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index 4c0b1f855..482e18d0d 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -2025,10 +2025,8 @@ static int atl1c_tso_csum(struct atl1c_adapter *adapter,
 						"IPV6 tso with zero data??\n");
 				goto check_sum;
 			} else
-				tcp_hdr(skb)->check = ~csum_ipv6_magic(
-						&ipv6_hdr(skb)->saddr,
-						&ipv6_hdr(skb)->daddr,
-						0, IPPROTO_TCP, 0);
+				tcp_v6_gso_csum_prep(skb, false);
+
 			etpd->word1 |= 1 << TPD_LSO_EN_SHIFT;
 			etpd->word1 |= 1 << TPD_LSO_VER_SHIFT;
 			etpd->pkt_len = cpu_to_le32(skb->len);
diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index 01a50a4b2..c301ad736 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -2504,12 +2504,7 @@ bnad_tso_prepare(struct bnad *bnad, struct sk_buff *skb)
 					   IPPROTO_TCP, 0);
 		BNAD_UPDATE_CTR(bnad, tso4);
 	} else {
-		struct ipv6hdr *ipv6h = ipv6_hdr(skb);
-
-		ipv6h->payload_len = 0;
-		tcp_hdr(skb)->check =
-			~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr, 0,
-					 IPPROTO_TCP, 0);
+		tcp_v6_gso_csum_prep(skb, true);
 		BNAD_UPDATE_CTR(bnad, tso6);
 	}
 
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index ddf60dc9a..683c628ef 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -696,8 +696,7 @@ static void enic_preload_tcp_csum(struct sk_buff *skb)
 		tcp_hdr(skb)->check = ~csum_tcpudp_magic(ip_hdr(skb)->saddr,
 			ip_hdr(skb)->daddr, 0, IPPROTO_TCP, 0);
 	} else if (skb->protocol == cpu_to_be16(ETH_P_IPV6)) {
-		tcp_hdr(skb)->check = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
-			&ipv6_hdr(skb)->daddr, 0, IPPROTO_TCP, 0);
+		tcp_v6_gso_csum_prep(skb, false);
 	}
 }
 
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 2bced34c1..0664985e8 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -2715,11 +2715,7 @@ static int e1000_tso(struct e1000_adapter *adapter,
 			cmd_length = E1000_TXD_CMD_IP;
 			ipcse = skb_transport_offset(skb) - 1;
 		} else if (skb_is_gso_v6(skb)) {
-			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);
+			tcp_v6_gso_csum_prep(skb, true);
 			ipcse = 0;
 		}
 		ipcss = skb_network_offset(skb);
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index db4ea58ba..7dda7d407 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5462,10 +5462,7 @@ static int e1000_tso(struct e1000_ring *tx_ring, struct sk_buff *skb,
 		cmd_length = E1000_TXD_CMD_IP;
 		ipcse = skb_transport_offset(skb) - 1;
 	} else if (skb_is_gso_v6(skb)) {
-		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);
+		tcp_v6_gso_csum_prep(skb, true);
 		ipcse = 0;
 	}
 	ipcss = skb_network_offset(skb);
diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
index 2e4975572..cde9be497 100644
--- a/drivers/net/ethernet/jme.c
+++ b/drivers/net/ethernet/jme.c
@@ -2077,12 +2077,7 @@ jme_tx_tso(struct sk_buff *skb, __le16 *mss, u8 *flags)
 								IPPROTO_TCP,
 								0);
 		} else {
-			struct ipv6hdr *ip6h = ipv6_hdr(skb);
-
-			tcp_hdr(skb)->check = ~csum_ipv6_magic(&ip6h->saddr,
-								&ip6h->daddr, 0,
-								IPPROTO_TCP,
-								0);
+			tcp_v6_gso_csum_prep(skb, false);
 		}
 
 		return 0;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index e452f4242..3d8469d97 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -632,10 +632,7 @@ static int ionic_tx_tcp_pseudo_csum(struct sk_buff *skb)
 					   ip_hdr(skb)->daddr,
 					   0, IPPROTO_TCP, 0);
 	} else if (skb->protocol == cpu_to_be16(ETH_P_IPV6)) {
-		tcp_hdr(skb)->check =
-			~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
-					 &ipv6_hdr(skb)->daddr,
-					 0, IPPROTO_TCP, 0);
+		tcp_v6_gso_csum_prep(skb, false);
 	}
 
 	return 0;
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index bebe38d74..01bcc5e68 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -1288,11 +1288,8 @@ static int emac_tso_csum(struct emac_adapter *adpt,
 			memset(tpd, 0, sizeof(*tpd));
 			memset(&extra_tpd, 0, sizeof(extra_tpd));
 
-			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);
+			tcp_v6_gso_csum_prep(skb, true);
+
 			TPD_PKT_LEN_SET(&extra_tpd, skb->len);
 			TPD_LSO_SET(&extra_tpd, 1);
 			TPD_LSOV_SET(&extra_tpd, 1);
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index e8224b543..d7a033053 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1148,11 +1148,7 @@ static netdev_tx_t netsec_netdev_start_xmit(struct sk_buff *skb,
 				~tcp_v4_check(0, ip_hdr(skb)->saddr,
 					      ip_hdr(skb)->daddr, 0);
 		} else {
-			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);
+			tcp_v6_gso_csum_prep(skb, true);
 		}
 
 		tx_ctrl.tcp_seg_offload_flag = true;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 65e12cb07..f41e48634 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -638,10 +638,7 @@ static int netvsc_xmit(struct sk_buff *skb, struct net_device *net, bool xdp_tx)
 		} 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);
+			tcp_v6_gso_csum_prep(skb, true);
 		}
 		lso_info->lso_v2_transmit.tcp_header_offset = skb_transport_offset(skb);
 		lso_info->lso_v2_transmit.mss = skb_shinfo(skb)->gso_size;
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 78ddbaf64..4ad2a1d42 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1948,29 +1948,6 @@ static void r8152_csum_workaround(struct r8152 *tp, struct sk_buff *skb,
 	}
 }
 
-/* msdn_giant_send_check()
- * According to the document of microsoft, the TCP Pseudo Header excludes the
- * packet length for IPv6 TCP large packets.
- */
-static int msdn_giant_send_check(struct sk_buff *skb)
-{
-	const struct ipv6hdr *ipv6h;
-	struct tcphdr *th;
-	int ret;
-
-	ret = skb_cow_head(skb, 0);
-	if (ret)
-		return ret;
-
-	ipv6h = ipv6_hdr(skb);
-	th = tcp_hdr(skb);
-
-	th->check = 0;
-	th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
-
-	return ret;
-}
-
 static inline void rtl_tx_vlan_tag(struct tx_desc *desc, struct sk_buff *skb)
 {
 	if (skb_vlan_tag_present(skb)) {
@@ -2016,10 +1993,11 @@ static int r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc,
 			break;
 
 		case htons(ETH_P_IPV6):
-			if (msdn_giant_send_check(skb)) {
+			if (skb_cow_head(skb, 0)) {
 				ret = TX_CSUM_TSO;
 				goto unavailable;
 			}
+			tcp_v6_gso_csum_prep(skb, false);
 			opts1 |= GTSENDV6;
 			break;
 
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 18f152fa0..92c2ecf3f 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -942,10 +942,7 @@ vmxnet3_prepare_tso(struct sk_buff *skb,
 		tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr, 0,
 						 IPPROTO_TCP, 0);
 	} else if (ctx->ipv6) {
-		struct ipv6hdr *iph = ipv6_hdr(skb);
-
-		tcph->check = ~csum_ipv6_magic(&iph->saddr, &iph->daddr, 0,
-					       IPPROTO_TCP, 0);
+		tcp_v6_gso_csum_prep(skb, false);
 	}
 }
 
-- 
2.25.0



^ permalink raw reply related

* RE: Is it safe for a NIC driver to use all the 48 bytes of skb->cb?
From: Haiyang Zhang @ 2020-02-17 22:31 UTC (permalink / raw)
  To: Dexuan Cui, Stephen Hemminger, David S. Miller,
	netdev@vger.kernel.org, KY Srinivasan,
	linux-kernel@vger.kernel.org
  Cc: linux-hyperv@vger.kernel.org
In-Reply-To: <HK0P153MB0148861FD9AAB88A98084206BF140@HK0P153MB0148.APCP153.PROD.OUTLOOK.COM>



> -----Original Message-----
> From: Dexuan Cui <decui@microsoft.com>
> Sent: Saturday, February 15, 2020 1:04 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; David S. Miller <davem@davemloft.net>;
> netdev@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; linux-
> kernel@vger.kernel.org
> Cc: linux-hyperv@vger.kernel.org
> Subject: RE: Is it safe for a NIC driver to use all the 48 bytes of skb->cb?
> 
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> > Sent: Saturday, February 15, 2020 7:20 AM
> > To: Dexuan Cui <decui@microsoft.com>; Stephen Hemminger
> >
> > According to the comments in skbuff.h below, it is the responsibility of the
> > owning layer to make a SKB clone, if it wants to keep the data across layers.
> > So, every layer can still use all of the 48 bytes.
> >
> >         /*
> >          * This is the control buffer. It is free to use for every
> >          * layer. Please put your private variables there. If you
> >          * want to keep them across layers you have to do a skb_clone()
> >          * first. This is owned by whoever has the skb queued ATM.
> >          */
> >         char                    cb[48] __aligned(8);
> >
> > > Now hv_netvsc assumes it can use all of the 48-bytes, though it uses only
> > > 20 bytes, but just in case the struct hv_netvsc_packet grows to >32 bytes in
> > the
> > > future, should we change the BUILD_BUG_ON() in netvsc_start_xmit() to
> > > BUILD_BUG_ON(sizeof(struct hv_netvsc_packet) > SKB_SGO_CB_OFFSET); ?
> >
> > Based on the explanation above, the existing hv_netvsc code is correct.
> >
> > Thanks,
> > - Haiyang
> 
> Got it. So if the upper layer saves something in the cb, it must do a skb_clone()
> and pass the new skb to hv_netvsc. hv_netvsc is the lowest layer in the network
> stack, so it can use all the 48 bytes without calling skb_clone().
> 
> BTW, now I happen to have a different question: in netvsc_probe() we have
> net->needed_headroom = RNDIS_AND_PPI_SIZE;
> I think this means when the network stack (ARP, IP, ICMP, TCP, UDP,etc) passes
> a
> skb to hv_netvsc, the skb's headroom is increased by an extra size of
> net->needed_headroom, right? Then in netvsc_xmit(), why do we still need to
> call skb_cow_head(skb, RNDIS_AND_PPI_SIZE)? -- this looks unnecessary to me?

skb_cow_head() only expands the headroom if it is not enough, in case some 
upper layer path didn't reserve enough.

> PS, what does the "cow" here mean? Copy On Write? It looks skb_cow_head()
> just copies the data (if necessary) and it has nothing to do with the
> write-protection in the MMU code.

Unrelated to MMU. It just copies some data to make room for writing.

Thanks,
- Haiyang

^ permalink raw reply

* Re: [PATCH net-next 1/3] net: core: add helper tcp_v6_gso_csum_prep
From: Alexander Duyck @ 2020-02-18 18:25 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc., netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <9fdc5f0c-fdf0-122e-48a5-43ff029cf8d9@gmail.com>

On Mon, Feb 17, 2020 at 1:41 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> Several network drivers for chips that support TSO6 share the same code
> for preparing the TCP header. A difference is that some reset the
> payload_len whilst others don't do this. Let's factor out this common
> code to a new helper.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  include/net/ip6_checksum.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
> index 7bec95df4..ef0130023 100644
> --- a/include/net/ip6_checksum.h
> +++ b/include/net/ip6_checksum.h
> @@ -76,6 +76,18 @@ static inline void __tcp_v6_send_check(struct sk_buff *skb,
>         }
>  }
>
> +static inline void tcp_v6_gso_csum_prep(struct sk_buff *skb,
> +                                       bool clear_payload_len)
> +{
> +       struct ipv6hdr *ipv6h = ipv6_hdr(skb);
> +       struct tcphdr *th = tcp_hdr(skb);
> +
> +       if (clear_payload_len)
> +               ipv6h->payload_len = 0;
> +
> +       th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
> +}
> +
>  #if IS_ENABLED(CONFIG_IPV6)
>  static inline void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
>  {

So functionally I believe this is correct. The only piece I have a
question about is if we should just force the clear_payload_len as
always being the case since the value should either be
ignored/overwritten in any GSO case anyway.

Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>

^ permalink raw reply

* Re: [PATCH net-next 3/3] net: use new helper tcp_v6_gso_csum_prep
From: Alexander Duyck @ 2020-02-18 18:34 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc., netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <9270ae4b-feb1-6a4d-8a22-fbe5e47b7617@gmail.com>

On Mon, Feb 17, 2020 at 1:43 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> Use new helper tcp_v6_gso_csum_prep in additional network drivers.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/ethernet/atheros/alx/main.c       |  5 +---
>  .../net/ethernet/atheros/atl1c/atl1c_main.c   |  6 ++---
>  drivers/net/ethernet/brocade/bna/bnad.c       |  7 +----
>  drivers/net/ethernet/cisco/enic/enic_main.c   |  3 +--
>  drivers/net/ethernet/intel/e1000/e1000_main.c |  6 +----
>  drivers/net/ethernet/intel/e1000e/netdev.c    |  5 +---
>  drivers/net/ethernet/jme.c                    |  7 +----
>  .../net/ethernet/pensando/ionic/ionic_txrx.c  |  5 +---
>  drivers/net/ethernet/qualcomm/emac/emac-mac.c |  7 ++---
>  drivers/net/ethernet/socionext/netsec.c       |  6 +----
>  drivers/net/hyperv/netvsc_drv.c               |  5 +---
>  drivers/net/usb/r8152.c                       | 26 ++-----------------
>  drivers/net/vmxnet3/vmxnet3_drv.c             |  5 +---
>  13 files changed, 16 insertions(+), 77 deletions(-)
>

It might make sense to break this up into several smaller patches
based on the maintainers for the various driver bits.

So the changes all look fine to me, but I am not that familiar with
the non-Intel drivers.

Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>

^ permalink raw reply

* Re: [PATCH net-next 2/3] r8169: use new helper tcp_v6_gso_csum_prep
From: Alexander Duyck @ 2020-02-18 18:37 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc., netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <02ea88e7-1a79-f779-d58c-bb1dced0b3b4@gmail.com>

On Mon, Feb 17, 2020 at 1:42 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> Simplify the code by using new helper tcp_v6_gso_csum_prep.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 26 ++---------------------
>  1 file changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 5a9143b50..75ba10069 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -4108,29 +4108,6 @@ static bool rtl_test_hw_pad_bug(struct rtl8169_private *tp, struct sk_buff *skb)
>         return skb->len < ETH_ZLEN && tp->mac_version == RTL_GIGA_MAC_VER_34;
>  }
>
> -/* msdn_giant_send_check()
> - * According to the document of microsoft, the TCP Pseudo Header excludes the
> - * packet length for IPv6 TCP large packets.
> - */
> -static int msdn_giant_send_check(struct sk_buff *skb)
> -{
> -       const struct ipv6hdr *ipv6h;
> -       struct tcphdr *th;
> -       int ret;
> -
> -       ret = skb_cow_head(skb, 0);
> -       if (ret)
> -               return ret;
> -
> -       ipv6h = ipv6_hdr(skb);
> -       th = tcp_hdr(skb);
> -
> -       th->check = 0;
> -       th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
> -
> -       return ret;
> -}
> -
>  static void rtl8169_tso_csum_v1(struct sk_buff *skb, u32 *opts)
>  {
>         u32 mss = skb_shinfo(skb)->gso_size;
> @@ -4163,9 +4140,10 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp,
>                         break;
>
>                 case htons(ETH_P_IPV6):
> -                       if (msdn_giant_send_check(skb))
> +                       if (skb_cow_head(skb, 0))
>                                 return false;
>
> +                       tcp_v6_gso_csum_prep(skb, false);
>                         opts[0] |= TD1_GTSENV6;
>                         break;
>

This change looks more or less identical to the one you made in
"drivers/net/usb/r8152.c" for patch 3. If you have to resubmit it
might make sense to pull that change out and include it here since
they are both essentially the same change.

^ permalink raw reply

* Re: [PATCH net-next 2/3] r8169: use new helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-18 18:45 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc., netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <CAKgT0UfaBpLxWQZO55-KE8QKJD9XgC2SCPAtzo=PA_MAwRxtuw@mail.gmail.com>

On 18.02.2020 19:37, Alexander Duyck wrote:
> On Mon, Feb 17, 2020 at 1:42 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> Simplify the code by using new helper tcp_v6_gso_csum_prep.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/net/ethernet/realtek/r8169_main.c | 26 ++---------------------
>>  1 file changed, 2 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>> index 5a9143b50..75ba10069 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -4108,29 +4108,6 @@ static bool rtl_test_hw_pad_bug(struct rtl8169_private *tp, struct sk_buff *skb)
>>         return skb->len < ETH_ZLEN && tp->mac_version == RTL_GIGA_MAC_VER_34;
>>  }
>>
>> -/* msdn_giant_send_check()
>> - * According to the document of microsoft, the TCP Pseudo Header excludes the
>> - * packet length for IPv6 TCP large packets.
>> - */
>> -static int msdn_giant_send_check(struct sk_buff *skb)
>> -{
>> -       const struct ipv6hdr *ipv6h;
>> -       struct tcphdr *th;
>> -       int ret;
>> -
>> -       ret = skb_cow_head(skb, 0);
>> -       if (ret)
>> -               return ret;
>> -
>> -       ipv6h = ipv6_hdr(skb);
>> -       th = tcp_hdr(skb);
>> -
>> -       th->check = 0;
>> -       th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
>> -
>> -       return ret;
>> -}
>> -
>>  static void rtl8169_tso_csum_v1(struct sk_buff *skb, u32 *opts)
>>  {
>>         u32 mss = skb_shinfo(skb)->gso_size;
>> @@ -4163,9 +4140,10 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp,
>>                         break;
>>
>>                 case htons(ETH_P_IPV6):
>> -                       if (msdn_giant_send_check(skb))
>> +                       if (skb_cow_head(skb, 0))
>>                                 return false;
>>
>> +                       tcp_v6_gso_csum_prep(skb, false);
>>                         opts[0] |= TD1_GTSENV6;
>>                         break;
>>
> 
> This change looks more or less identical to the one you made in
> "drivers/net/usb/r8152.c" for patch 3. If you have to resubmit it
> might make sense to pull that change out and include it here since
> they are both essentially the same change.
> 
Right, it's the same change. I just treated r8169 separately because
I happen to be maintainer of it.

^ permalink raw reply

* Re: [PATCH net-next 3/3] net: use new helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-18 18:54 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc., netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <CAKgT0UdP78GGnowWC85YiTAHOr63NiLa25=2TSckKBEzGBdeJA@mail.gmail.com>

On 18.02.2020 19:34, Alexander Duyck wrote:
> On Mon, Feb 17, 2020 at 1:43 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> Use new helper tcp_v6_gso_csum_prep in additional network drivers.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/net/ethernet/atheros/alx/main.c       |  5 +---
>>  .../net/ethernet/atheros/atl1c/atl1c_main.c   |  6 ++---
>>  drivers/net/ethernet/brocade/bna/bnad.c       |  7 +----
>>  drivers/net/ethernet/cisco/enic/enic_main.c   |  3 +--
>>  drivers/net/ethernet/intel/e1000/e1000_main.c |  6 +----
>>  drivers/net/ethernet/intel/e1000e/netdev.c    |  5 +---
>>  drivers/net/ethernet/jme.c                    |  7 +----
>>  .../net/ethernet/pensando/ionic/ionic_txrx.c  |  5 +---
>>  drivers/net/ethernet/qualcomm/emac/emac-mac.c |  7 ++---
>>  drivers/net/ethernet/socionext/netsec.c       |  6 +----
>>  drivers/net/hyperv/netvsc_drv.c               |  5 +---
>>  drivers/net/usb/r8152.c                       | 26 ++-----------------
>>  drivers/net/vmxnet3/vmxnet3_drv.c             |  5 +---
>>  13 files changed, 16 insertions(+), 77 deletions(-)
>>
> 
> It might make sense to break this up into several smaller patches
> based on the maintainers for the various driver bits.
> 
OK

> So the changes all look fine to me, but I am not that familiar with
> the non-Intel drivers.
> 
> Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 


^ permalink raw reply

* Re: [PATCH net-next 1/3] net: core: add helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-18 18:53 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc., netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <CAKgT0UeUEcoKZsRnxzftMA4tc2chasmW+sWQkP11hVLbdYTYxA@mail.gmail.com>

On 18.02.2020 19:25, Alexander Duyck wrote:
> On Mon, Feb 17, 2020 at 1:41 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> Several network drivers for chips that support TSO6 share the same code
>> for preparing the TCP header. A difference is that some reset the
>> payload_len whilst others don't do this. Let's factor out this common
>> code to a new helper.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  include/net/ip6_checksum.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
>> index 7bec95df4..ef0130023 100644
>> --- a/include/net/ip6_checksum.h
>> +++ b/include/net/ip6_checksum.h
>> @@ -76,6 +76,18 @@ static inline void __tcp_v6_send_check(struct sk_buff *skb,
>>         }
>>  }
>>
>> +static inline void tcp_v6_gso_csum_prep(struct sk_buff *skb,
>> +                                       bool clear_payload_len)
>> +{
>> +       struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>> +       struct tcphdr *th = tcp_hdr(skb);
>> +
>> +       if (clear_payload_len)
>> +               ipv6h->payload_len = 0;
>> +
>> +       th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
>> +}
>> +
>>  #if IS_ENABLED(CONFIG_IPV6)
>>  static inline void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
>>  {
> 
> So functionally I believe this is correct. The only piece I have a
> question about is if we should just force the clear_payload_len as
> always being the case since the value should either be
> ignored/overwritten in any GSO case anyway.
> 
I also thought about this and just wasn't sure whether this functional
change may break any driver. But yes, then let's change it this way.
Breaking down the series into smaller patches makes it easier to
revert an individual patch in case of a problem.

> Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
Thanks for the review.

^ permalink raw reply

* [PATCH net-next v2 0/13] net: core: add helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-18 19:55 UTC (permalink / raw)
  To: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc.
  Cc: netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List

Several network drivers for chips that support TSO6 share the same code
for preparing the TCP header, so let's factor it out to a helper.
A difference is that some drivers reset the payload_len whilst others
don't do this. This value is overwritten by TSO anyway, therefore
the new helper resets it in general.

Heiner Kallweit (13):
  net: core: add helper tcp_v6_gso_csum_prep
  r8169: use new helper tcp_v6_gso_csum_prep
  net: atheros: use new helper tcp_v6_gso_csum_prep
  bna: use new helper tcp_v6_gso_csum_prep
  enic: use new helper tcp_v6_gso_csum_prep
  e1000(e): use new helper tcp_v6_gso_csum_prep
  jme: use new helper tcp_v6_gso_csum_prep
  ionic: use new helper tcp_v6_gso_csum_prep
  net: qcom/emac: use new helper tcp_v6_gso_csum_prep
  net: socionext: use new helper tcp_v6_gso_csum_prep
  hv_netvsc: use new helper tcp_v6_gso_csum_prep
  r8152: use new helper tcp_v6_gso_csum_prep
  vmxnet3: use new helper tcp_v6_gso_csum_prep

 drivers/net/ethernet/atheros/alx/main.c       |  5 +---
 .../net/ethernet/atheros/atl1c/atl1c_main.c   |  6 ++---
 drivers/net/ethernet/brocade/bna/bnad.c       |  7 +----
 drivers/net/ethernet/cisco/enic/enic_main.c   |  3 +--
 drivers/net/ethernet/intel/e1000/e1000_main.c |  6 +----
 drivers/net/ethernet/intel/e1000e/netdev.c    |  5 +---
 drivers/net/ethernet/jme.c                    |  7 +----
 .../net/ethernet/pensando/ionic/ionic_txrx.c  |  5 +---
 drivers/net/ethernet/qualcomm/emac/emac-mac.c |  7 ++---
 drivers/net/ethernet/realtek/r8169_main.c     | 26 ++-----------------
 drivers/net/ethernet/socionext/netsec.c       |  6 +----
 drivers/net/hyperv/netvsc_drv.c               |  5 +---
 drivers/net/usb/r8152.c                       | 26 ++-----------------
 drivers/net/vmxnet3/vmxnet3_drv.c             |  5 +---
 include/net/ip6_checksum.h                    |  9 +++++++
 15 files changed, 27 insertions(+), 101 deletions(-)

-- 
2.25.1


^ permalink raw reply

* [PATCH net-next v2 11/13] hv_netvsc: use new helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-18 20:11 UTC (permalink / raw)
  To: David Miller, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Sasha Levin
  Cc: netdev@vger.kernel.org, Linux Kernel Mailing List, linux-hyperv
In-Reply-To: <fffc8b6d-68ed-7501-18f1-94cf548821fb@gmail.com>

Use new helper tcp_v6_gso_csum_prep in additional network drivers.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/hyperv/netvsc_drv.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 65e12cb07..5ee282b20 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -638,10 +638,7 @@ static int netvsc_xmit(struct sk_buff *skb, struct net_device *net, bool xdp_tx)
 		} 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);
+			tcp_v6_gso_csum_prep(skb);
 		}
 		lso_info->lso_v2_transmit.tcp_header_offset = skb_transport_offset(skb);
 		lso_info->lso_v2_transmit.mss = skb_shinfo(skb)->gso_size;
-- 
2.25.1



^ permalink raw reply related

* [PATCH net-next v2 01/13] net: core: add helper tcp_v6_gso_csum_prep
From: Heiner Kallweit @ 2020-02-18 19:56 UTC (permalink / raw)
  To: David Miller, Realtek linux nic maintainers, Jay Cliburn,
	Chris Snook, Rasesh Mody, Sudarsana Kalluru, GR-Linux-NIC-Dev,
	Christian Benvenuti, Govindarajulu Varadarajan, Parvi Kaustubhi,
	Jeff Kirsher, Guo-Fu Tseng, Shannon Nelson, Pensando Drivers,
	Timur Tabi, Jassi Brar, Ilias Apalodimas, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Sasha Levin, Ronak Doshi,
	VMware, Inc.
  Cc: netdev@vger.kernel.org, Linux Kernel Mailing List,
	intel-wired-lan, linux-hyperv, Linux USB Mailing List
In-Reply-To: <fffc8b6d-68ed-7501-18f1-94cf548821fb@gmail.com>

Several network drivers for chips that support TSO6 share the same code
for preparing the TCP header, so let's factor it out to a helper.
A difference is that some drivers reset the payload_len whilst others
don't do this. This value is overwritten by TSO anyway, therefore
the new helper resets it in general.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
 include/net/ip6_checksum.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
index 7bec95df4..27ec612cd 100644
--- a/include/net/ip6_checksum.h
+++ b/include/net/ip6_checksum.h
@@ -76,6 +76,15 @@ static inline void __tcp_v6_send_check(struct sk_buff *skb,
 	}
 }
 
+static inline void tcp_v6_gso_csum_prep(struct sk_buff *skb)
+{
+	struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+	struct tcphdr *th = tcp_hdr(skb);
+
+	ipv6h->payload_len = 0;
+	th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 static inline void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
 {
-- 
2.25.1



^ permalink raw reply related

* Re: [PATCH net-next v2 0/13] net: core: add helper tcp_v6_gso_csum_prep
From: David Miller @ 2020-02-19 19:22 UTC (permalink / raw)
  To: hkallweit1
  Cc: nic_swsd, jcliburn, chris.snook, rmody, skalluru,
	GR-Linux-NIC-Dev, benve, _govind, pkaustub, jeffrey.t.kirsher,
	cooldavid, snelson, drivers, timur, jaswinder.singh,
	ilias.apalodimas, kys, haiyangz, sthemmin, sashal, doshir,
	pv-drivers, netdev, linux-kernel, intel-wired-lan, linux-hyperv,
	linux-usb
In-Reply-To: <fffc8b6d-68ed-7501-18f1-94cf548821fb@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 18 Feb 2020 20:55:18 +0100

> Several network drivers for chips that support TSO6 share the same code
> for preparing the TCP header, so let's factor it out to a helper.
> A difference is that some drivers reset the payload_len whilst others
> don't do this. This value is overwritten by TSO anyway, therefore
> the new helper resets it in general.

Series applied, thanks Heiner.

^ permalink raw reply

* Re: [PATCH AUTOSEL 5.5 389/542] clocksource/drivers/hyper-v: Reserve PAGE_SIZE space for tsc page
From: Sasha Levin @ 2020-02-20 17:34 UTC (permalink / raw)
  To: Michael Kelley
  Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Boqun Feng,
	linux-hyperv@vger.kernel.org, Daniel Lezcano
In-Reply-To: <MW2PR2101MB10526693E43DF07E03D5EB0FD7150@MW2PR2101MB1052.namprd21.prod.outlook.com>

On Fri, Feb 14, 2020 at 04:11:03PM +0000, Michael Kelley wrote:
>This patch does not need to be backported to any stable releases.  It is prep work for guests on Hyper-V ARM64 when the guest page size is 16K or 64K, and that functionality isn't upstream yet.

Now dropped, thank you.

-- 
Thanks,
Sasha

^ permalink raw reply

* Re: [PATCH v3 0/3] PCI: hv: Generify pci-hyperv.c
From: Boqun Feng @ 2020-02-21  2:33 UTC (permalink / raw)
  To: linux-pci, linux-hyperv, linux-kernel, linux-arm-kernel
  Cc: Michael Kelley, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Sasha Levin, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Lorenzo Pieralisi,
	Andrew Murray, Bjorn Helgaas
In-Reply-To: <20200210033953.99692-1-boqun.feng@gmail.com>

Ping ;-)

Any suggestion or plan on this patchset?

Thanks and Regards,
Boqun

On Mon, Feb 10, 2020 at 11:39:50AM +0800, Boqun Feng wrote:
> Hi,
> 
> This is the first part for virtual PCI support of Hyper-V guest on
> ARM64. The whole patchset doesn't have any functional change, but only
> refactors the pci-hyperv.c code to make it more arch-independent.
> 
> Previous version:
> v1: https://lore.kernel.org/lkml/20200121015713.69691-1-boqun.feng@gmail.com/
> v2: https://lore.kernel.org/linux-arm-kernel/20200203050313.69247-1-boqun.feng@gmail.com/
> 
> Changes since v2:
> 
> *	Rebased on 5.6-rc1
> 
> *	Reword commit logs as per Andrew's suggestion.
> 
> *	It makes more sense to have a generic interface to set the whole
> 	msi_entry rather than only the "address" field. So change
> 	hv_set_msi_address_from_desc() to hv_set_msi_entry_from_desc().
> 	Additionally, make it an inline function as per the suggestion
> 	of Andrew and Thomas.
> 
> *	Add the missing comment saying the partition_id of
> 	hv_retarget_device_interrupt must be self.
> 
> *	Add the explanation for why "__packed" is needed for TLFS
> 	structures.
> 
> I've done compile and boot test of this patchset, also done some tests
> with a pass-through NVMe device.
> 
> Suggestions and comments are welcome!
> 
> Regards,
> Boqun
> 
> Boqun Feng (3):
>   PCI: hv: Move hypercall related definitions into tlfs header
>   PCI: hv: Move retarget related structures into tlfs header
>   PCI: hv: Introduce hv_msi_entry
> 
>  arch/x86/include/asm/hyperv-tlfs.h  | 41 +++++++++++++++++++++++++++
>  arch/x86/include/asm/mshyperv.h     |  8 ++++++
>  drivers/pci/controller/pci-hyperv.c | 43 ++---------------------------
>  3 files changed, 52 insertions(+), 40 deletions(-)
> 
> -- 
> 2.24.1
> 

^ permalink raw reply

* Re: [PATCH v3 0/3] PCI: hv: Generify pci-hyperv.c
From: Lorenzo Pieralisi @ 2020-02-21 10:44 UTC (permalink / raw)
  To: Boqun Feng
  Cc: linux-pci, linux-hyperv, linux-kernel, linux-arm-kernel,
	Michael Kelley, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Sasha Levin, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Andrew Murray,
	Bjorn Helgaas
In-Reply-To: <20200221023344.GJ69864@debian-boqun.qqnc3lrjykvubdpftowmye0fmh.lx.internal.cloudapp.net>

On Fri, Feb 21, 2020 at 10:33:44AM +0800, Boqun Feng wrote:
> Ping ;-)
> 
> Any suggestion or plan on this patchset?

Hi,

I shall have a look shortly, thanks.

Lorenzo

> Thanks and Regards,
> Boqun
> 
> On Mon, Feb 10, 2020 at 11:39:50AM +0800, Boqun Feng wrote:
> > Hi,
> > 
> > This is the first part for virtual PCI support of Hyper-V guest on
> > ARM64. The whole patchset doesn't have any functional change, but only
> > refactors the pci-hyperv.c code to make it more arch-independent.
> > 
> > Previous version:
> > v1: https://lore.kernel.org/lkml/20200121015713.69691-1-boqun.feng@gmail.com/
> > v2: https://lore.kernel.org/linux-arm-kernel/20200203050313.69247-1-boqun.feng@gmail.com/
> > 
> > Changes since v2:
> > 
> > *	Rebased on 5.6-rc1
> > 
> > *	Reword commit logs as per Andrew's suggestion.
> > 
> > *	It makes more sense to have a generic interface to set the whole
> > 	msi_entry rather than only the "address" field. So change
> > 	hv_set_msi_address_from_desc() to hv_set_msi_entry_from_desc().
> > 	Additionally, make it an inline function as per the suggestion
> > 	of Andrew and Thomas.
> > 
> > *	Add the missing comment saying the partition_id of
> > 	hv_retarget_device_interrupt must be self.
> > 
> > *	Add the explanation for why "__packed" is needed for TLFS
> > 	structures.
> > 
> > I've done compile and boot test of this patchset, also done some tests
> > with a pass-through NVMe device.
> > 
> > Suggestions and comments are welcome!
> > 
> > Regards,
> > Boqun
> > 
> > Boqun Feng (3):
> >   PCI: hv: Move hypercall related definitions into tlfs header
> >   PCI: hv: Move retarget related structures into tlfs header
> >   PCI: hv: Introduce hv_msi_entry
> > 
> >  arch/x86/include/asm/hyperv-tlfs.h  | 41 +++++++++++++++++++++++++++
> >  arch/x86/include/asm/mshyperv.h     |  8 ++++++
> >  drivers/pci/controller/pci-hyperv.c | 43 ++---------------------------
> >  3 files changed, 52 insertions(+), 40 deletions(-)
> > 
> > -- 
> > 2.24.1
> > 

^ permalink raw reply

* Re: [PATCH] PCI: hv: Use kfree(hbus) in hv_pci_probe()'s error handling path
From: Lorenzo Pieralisi @ 2020-02-21 14:40 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: kys, haiyangz, sthemmin, sashal, bhelgaas, linux-hyperv,
	linux-pci, linux-kernel, mikelley, Alexander.Levin
In-Reply-To: <1578350351-129783-1-git-send-email-decui@microsoft.com>

On Mon, Jan 06, 2020 at 02:39:11PM -0800, Dexuan Cui wrote:
> Now that we use kzalloc() to allocate the hbus buffer, we should use
> kfree() in the error path as well.
> 
> Also remove the type casting, since it's unnecessary in C.

Two unrelated logical changes -> two patches please, I know it is
tempting but it is important to split logical changes into separate
patches.

Thanks,
Lorenzo

> Fixes: 877b911a5ba0 ("PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer")
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> 
> Sorry for missing the error handling path.
> 
>  drivers/pci/controller/pci-hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 9977abff92fc..15011a349520 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2922,7 +2922,7 @@ static int hv_pci_probe(struct hv_device *hdev,
>  	 * positive by using kmemleak_alloc() and kmemleak_free() to ask
>  	 * kmemleak to track and scan the hbus buffer.
>  	 */
> -	hbus = (struct hv_pcibus_device *)kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> +	hbus = kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
>  	if (!hbus)
>  		return -ENOMEM;
>  	hbus->state = hv_pcibus_init;
> @@ -3058,7 +3058,7 @@ static int hv_pci_probe(struct hv_device *hdev,
>  free_dom:
>  	hv_put_dom_num(hbus->sysdata.domain);
>  free_bus:
> -	free_page((unsigned long)hbus);
> +	kfree(hbus);
>  	return ret;
>  }
>  
> -- 
> 2.19.1
> 

^ permalink raw reply

* [net PATCH] hv_netvsc: Fix unwanted wakeup in netvsc_attach()
From: Haiyang Zhang @ 2020-02-21 16:32 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel

When netvsc_attach() is called by operations like changing MTU, etc.,
an extra wakeup may happen while netvsc_attach() calling
rndis_filter_device_add() which sends rndis messages when queue is
stopped in netvsc_detach(). The completion message will wake up queue 0.

We can reproduce the issue by changing MTU etc., then the wake_queue
counter from "ethtool -S" will increase beyond stop_queue counter:
     stop_queue: 0
     wake_queue: 1
The issue causes queue wake up, and counter increment, no other ill
effects in current code. So we didn't see any network problem for now.

To fix this, initialize tx_disable to true, and set it to false when
the NIC is ready to be attached or registered.

Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>

---
 drivers/net/hyperv/netvsc.c     | 2 +-
 drivers/net/hyperv/netvsc_drv.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index ae3f308..1b320bc 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -99,7 +99,7 @@ static struct netvsc_device *alloc_net_device(void)
 
 	init_waitqueue_head(&net_device->wait_drain);
 	net_device->destroy = false;
-	net_device->tx_disable = false;
+	net_device->tx_disable = true;
 
 	net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
 	net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 65e12cb..2c0a24c 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1068,6 +1068,7 @@ static int netvsc_attach(struct net_device *ndev,
 	}
 
 	/* In any case device is now ready */
+	nvdev->tx_disable = false;
 	netif_device_attach(ndev);
 
 	/* Note: enable and attach happen when sub-channels setup */
@@ -2476,6 +2477,8 @@ static int netvsc_probe(struct hv_device *dev,
 	else
 		net->max_mtu = ETH_DATA_LEN;
 
+	nvdev->tx_disable = false;
+
 	ret = register_netdevice(net);
 	if (ret != 0) {
 		pr_err("Unable to register netdev.\n");
-- 
1.8.3.1


^ permalink raw reply related

* RE: [PATCH] PCI: hv: Use kfree(hbus) in hv_pci_probe()'s error handling path
From: Dexuan Cui @ 2020-02-21 20:17 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	sashal@kernel.org, bhelgaas@google.com,
	linux-hyperv@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, Michael Kelley, Sasha Levin
In-Reply-To: <20200221144003.GD15440@e121166-lin.cambridge.arm.com>

> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Lorenzo Pieralisi
> Sent: Friday, February 21, 2020 6:40 AM
> 
> On Mon, Jan 06, 2020 at 02:39:11PM -0800, Dexuan Cui wrote:
> > Now that we use kzalloc() to allocate the hbus buffer, we should use
> > kfree() in the error path as well.
> >
> > Also remove the type casting, since it's unnecessary in C.
> 
> Two unrelated logical changes -> two patches please, I know it is
> tempting but it is important to split logical changes into separate
> patches.
> 
> Thanks,
> Lorenzo

Ok, will post 2 separate patches shortly.

Thanks,
-- Dexuan

^ permalink raw reply

* [PATCH v2 1/2] PCI: hv: Remove unnecessary type casting from kzalloc
From: Dexuan Cui @ 2020-02-22  5:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, lorenzo.pieralisi, bhelgaas,
	linux-hyperv, linux-pci, linux-kernel, mikelley
  Cc: Dexuan Cui

In C, there is no need to cast a void * to any other pointer type.

Fixes: 877b911a5ba0 ("PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---

Change in v2: this was part of v1.

 drivers/pci/controller/pci-hyperv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 9977abff92fc..0fe0283368d2 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2922,7 +2922,7 @@ static int hv_pci_probe(struct hv_device *hdev,
 	 * positive by using kmemleak_alloc() and kmemleak_free() to ask
 	 * kmemleak to track and scan the hbus buffer.
 	 */
-	hbus = (struct hv_pcibus_device *)kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
+	hbus = kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
 	if (!hbus)
 		return -ENOMEM;
 	hbus->state = hv_pcibus_init;
-- 
2.19.1


^ permalink raw reply related

* [PATCH v2 2/2] PCI: hv: Use kfree(hbus) in hv_pci_probe()'s error handling path
From: Dexuan Cui @ 2020-02-22  5:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, lorenzo.pieralisi, bhelgaas,
	linux-hyperv, linux-pci, linux-kernel, mikelley
  Cc: Dexuan Cui
In-Reply-To: <1582351197-12303-1-git-send-email-decui@microsoft.com>

Now that we use kzalloc() to allocate the hbus buffer, we should use
kfree() in the error path as well.

Fixes: 877b911a5ba0 ("PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
 drivers/pci/controller/pci-hyperv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Change in v2: this was part of v1.

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 0fe0283368d2..15011a349520 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -3058,7 +3058,7 @@ static int hv_pci_probe(struct hv_device *hdev,
 free_dom:
 	hv_put_dom_num(hbus->sysdata.domain);
 free_bus:
-	free_page((unsigned long)hbus);
+	kfree(hbus);
 	return ret;
 }
 
-- 
2.19.1


^ permalink raw reply related

* Re: [net PATCH] hv_netvsc: Fix unwanted wakeup in netvsc_attach()
From: David Miller @ 2020-02-24  0:33 UTC (permalink / raw)
  To: haiyangz
  Cc: sashal, linux-hyperv, netdev, kys, sthemmin, olaf, vkuznets,
	linux-kernel
In-Reply-To: <1582302738-24352-1-git-send-email-haiyangz@microsoft.com>

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Fri, 21 Feb 2020 08:32:18 -0800

> When netvsc_attach() is called by operations like changing MTU, etc.,
> an extra wakeup may happen while netvsc_attach() calling
> rndis_filter_device_add() which sends rndis messages when queue is
> stopped in netvsc_detach(). The completion message will wake up queue 0.
> 
> We can reproduce the issue by changing MTU etc., then the wake_queue
> counter from "ethtool -S" will increase beyond stop_queue counter:
>      stop_queue: 0
>      wake_queue: 1
> The issue causes queue wake up, and counter increment, no other ill
> effects in current code. So we didn't see any network problem for now.
> 
> To fix this, initialize tx_disable to true, and set it to false when
> the NIC is ready to be attached or registered.
> 
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH v2 1/2] PCI: hv: Remove unnecessary type casting from kzalloc
From: Lorenzo Pieralisi @ 2020-02-24 12:04 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: kys, haiyangz, sthemmin, bhelgaas, linux-hyperv, linux-pci,
	linux-kernel, mikelley
In-Reply-To: <1582351197-12303-1-git-send-email-decui@microsoft.com>

On Fri, Feb 21, 2020 at 09:59:56PM -0800, Dexuan Cui wrote:
> In C, there is no need to cast a void * to any other pointer type.
> 
> Fixes: 877b911a5ba0 ("PCI: hv: Avoid a kmemleak false positive caused by the hbus buffer")

This patch fixes nothing, anyway, that's a leftover that I removed.

Applied the series to pci/hv for v5.7.

Thanks,
Lorenzo

> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> ---
> 
> Change in v2: this was part of v1.
> 
>  drivers/pci/controller/pci-hyperv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 9977abff92fc..0fe0283368d2 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2922,7 +2922,7 @@ static int hv_pci_probe(struct hv_device *hdev,
>  	 * positive by using kmemleak_alloc() and kmemleak_free() to ask
>  	 * kmemleak to track and scan the hbus buffer.
>  	 */
> -	hbus = (struct hv_pcibus_device *)kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> +	hbus = kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
>  	if (!hbus)
>  		return -ENOMEM;
>  	hbus->state = hv_pcibus_init;
> -- 
> 2.19.1
> 

^ permalink raw reply

* Re: [Patch v4 2/2] PCI: hv: Add support for protocol 1.3 and support PCI_BUS_RELATIONS2
From: Lorenzo Pieralisi @ 2020-02-25 10:36 UTC (permalink / raw)
  To: longli
  Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	Andrew Murray, Bjorn Helgaas, linux-hyperv, linux-pci,
	linux-kernel, Long Li
In-Reply-To: <1578946101-74036-2-git-send-email-longli@linuxonhyperv.com>

On Mon, Jan 13, 2020 at 12:08:21PM -0800, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
> 
> Starting with Hyper-V PCI protocol version 1.3, the host VSP can send
> PCI_BUS_RELATIONS2 and pass the vNUMA node information for devices on the
> bus. The vNUMA node tells which guest NUMA node this device is on based
> on guest VM configuration topology and physical device inforamtion.

s/inforamtion/information

Please rebase this series on top of my pci/hv branch, it does
not apply, I will merge it then.

Thanks,
Lorenzo

> Add code to negotiate v1.3 and process PCI_BUS_RELATIONS2.
> 
> Signed-off-by: Long Li <longli@microsoft.com>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> ---
> Changes
> v2: Changed some spaces to tabs, added put_pcichild() after get_pcichild_wslot(), renamed pci_assign_numa_node() to hv_pci_assign_numa_node()
> v4: Fixed spelling
> 
>  drivers/pci/controller/pci-hyperv.c | 109 ++++++++++++++++++++++++++++
>  1 file changed, 109 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 3b3e1967cf08..147358fae8a2 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -63,6 +63,7 @@
>  enum pci_protocol_version_t {
>  	PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1),	/* Win10 */
>  	PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2),	/* RS1 */
> +	PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3),	/* Vibranium */
>  };
>  
>  #define CPU_AFFINITY_ALL	-1ULL
> @@ -72,6 +73,7 @@ enum pci_protocol_version_t {
>   * first.
>   */
>  static enum pci_protocol_version_t pci_protocol_versions[] = {
> +	PCI_PROTOCOL_VERSION_1_3,
>  	PCI_PROTOCOL_VERSION_1_2,
>  	PCI_PROTOCOL_VERSION_1_1,
>  };
> @@ -124,6 +126,7 @@ enum pci_message_type {
>  	PCI_RESOURCES_ASSIGNED2		= PCI_MESSAGE_BASE + 0x16,
>  	PCI_CREATE_INTERRUPT_MESSAGE2	= PCI_MESSAGE_BASE + 0x17,
>  	PCI_DELETE_INTERRUPT_MESSAGE2	= PCI_MESSAGE_BASE + 0x18, /* unused */
> +	PCI_BUS_RELATIONS2		= PCI_MESSAGE_BASE + 0x19,
>  	PCI_MESSAGE_MAXIMUM
>  };
>  
> @@ -169,6 +172,26 @@ struct pci_function_description {
>  	u32	ser;	/* serial number */
>  } __packed;
>  
> +enum pci_device_description_flags {
> +	HV_PCI_DEVICE_FLAG_NONE			= 0x0,
> +	HV_PCI_DEVICE_FLAG_NUMA_AFFINITY	= 0x1,
> +};
> +
> +struct pci_function_description2 {
> +	u16	v_id;	/* vendor ID */
> +	u16	d_id;	/* device ID */
> +	u8	rev;
> +	u8	prog_intf;
> +	u8	subclass;
> +	u8	base_class;
> +	u32	subsystem_id;
> +	union	win_slot_encoding win_slot;
> +	u32	ser;	/* serial number */
> +	u32	flags;
> +	u16	virtual_numa_node;
> +	u16	reserved;
> +} __packed;
> +
>  /**
>   * struct hv_msi_desc
>   * @vector:		IDT entry
> @@ -304,6 +327,12 @@ struct pci_bus_relations {
>  	struct pci_function_description func[0];
>  } __packed;
>  
> +struct pci_bus_relations2 {
> +	struct pci_incoming_message incoming;
> +	u32 device_count;
> +	struct pci_function_description2 func[0];
> +} __packed;
> +
>  struct pci_q_res_req_response {
>  	struct vmpacket_descriptor hdr;
>  	s32 status;			/* negative values are failures */
> @@ -1417,6 +1446,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>  		break;
>  
>  	case PCI_PROTOCOL_VERSION_1_2:
> +	case PCI_PROTOCOL_VERSION_1_3:
>  		size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2,
>  					dest,
>  					hpdev->desc.win_slot.slot,
> @@ -1798,6 +1828,27 @@ static void hv_pci_remove_slots(struct hv_pcibus_device *hbus)
>  	}
>  }
>  
> +/*
> + * Set NUMA node for the devices on the bus
> + */
> +static void hv_pci_assign_numa_node(struct hv_pcibus_device *hbus)
> +{
> +	struct pci_dev *dev;
> +	struct pci_bus *bus = hbus->pci_bus;
> +	struct hv_pci_dev *hv_dev;
> +
> +	list_for_each_entry(dev, &bus->devices, bus_list) {
> +		hv_dev = get_pcichild_wslot(hbus, devfn_to_wslot(dev->devfn));
> +		if (!hv_dev)
> +			continue;
> +
> +		if (hv_dev->desc.flags & HV_PCI_DEVICE_FLAG_NUMA_AFFINITY)
> +			set_dev_node(&dev->dev, hv_dev->desc.virtual_numa_node);
> +
> +		put_pcichild(hv_dev);
> +	}
> +}
> +
>  /**
>   * create_root_hv_pci_bus() - Expose a new root PCI bus
>   * @hbus:	Root PCI bus, as understood by this driver
> @@ -1820,6 +1871,7 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
>  
>  	pci_lock_rescan_remove();
>  	pci_scan_child_bus(hbus->pci_bus);
> +	hv_pci_assign_numa_node(hbus);
>  	pci_bus_assign_resources(hbus->pci_bus);
>  	hv_pci_assign_slots(hbus);
>  	pci_bus_add_devices(hbus->pci_bus);
> @@ -2088,6 +2140,7 @@ static void pci_devices_present_work(struct work_struct *work)
>  		 */
>  		pci_lock_rescan_remove();
>  		pci_scan_child_bus(hbus->pci_bus);
> +		hv_pci_assign_numa_node(hbus);
>  		hv_pci_assign_slots(hbus);
>  		pci_unlock_rescan_remove();
>  		break;
> @@ -2184,6 +2237,46 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus,
>  		kfree(dr);
>  }
>  
> +/**
> + * hv_pci_devices_present2() - Handle list of new children
> + * @hbus:	Root PCI bus, as understood by this driver
> + * @relations2:	Packet from host listing children
> + *
> + * This function is the v2 version of hv_pci_devices_present()
> + */
> +static void hv_pci_devices_present2(struct hv_pcibus_device *hbus,
> +				    struct pci_bus_relations2 *relations)
> +{
> +	struct hv_dr_state *dr;
> +	int i;
> +
> +	dr = kzalloc(offsetof(struct hv_dr_state, func) +
> +		     (sizeof(struct hv_pcidev_description) *
> +		      (relations->device_count)), GFP_NOWAIT);
> +
> +	if (!dr)
> +		return;
> +
> +	dr->device_count = relations->device_count;
> +	for (i = 0; i < dr->device_count; i++) {
> +		dr->func[i].v_id = relations->func[i].v_id;
> +		dr->func[i].d_id = relations->func[i].d_id;
> +		dr->func[i].rev = relations->func[i].rev;
> +		dr->func[i].prog_intf = relations->func[i].prog_intf;
> +		dr->func[i].subclass = relations->func[i].subclass;
> +		dr->func[i].base_class = relations->func[i].base_class;
> +		dr->func[i].subsystem_id = relations->func[i].subsystem_id;
> +		dr->func[i].win_slot = relations->func[i].win_slot;
> +		dr->func[i].ser = relations->func[i].ser;
> +		dr->func[i].flags = relations->func[i].flags;
> +		dr->func[i].virtual_numa_node =
> +			relations->func[i].virtual_numa_node;
> +	}
> +
> +	if (hv_pci_start_relations_work(hbus, dr))
> +		kfree(dr);
> +}
> +
>  /**
>   * hv_eject_device_work() - Asynchronously handles ejection
>   * @work:	Work struct embedded in internal device struct
> @@ -2289,6 +2382,7 @@ static void hv_pci_onchannelcallback(void *context)
>  	struct pci_response *response;
>  	struct pci_incoming_message *new_message;
>  	struct pci_bus_relations *bus_rel;
> +	struct pci_bus_relations2 *bus_rel2;
>  	struct pci_dev_inval_block *inval;
>  	struct pci_dev_incoming *dev_message;
>  	struct hv_pci_dev *hpdev;
> @@ -2356,6 +2450,21 @@ static void hv_pci_onchannelcallback(void *context)
>  				hv_pci_devices_present(hbus, bus_rel);
>  				break;
>  
> +			case PCI_BUS_RELATIONS2:
> +
> +				bus_rel2 = (struct pci_bus_relations2 *)buffer;
> +				if (bytes_recvd <
> +				    offsetof(struct pci_bus_relations2, func) +
> +				    (sizeof(struct pci_function_description2) *
> +				     (bus_rel2->device_count))) {
> +					dev_err(&hbus->hdev->device,
> +						"bus relations v2 too small\n");
> +					break;
> +				}
> +
> +				hv_pci_devices_present2(hbus, bus_rel2);
> +				break;
> +
>  			case PCI_EJECT:
>  
>  				dev_message = (struct pci_dev_incoming *)buffer;
> -- 
> 2.17.1
> 

^ 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