Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH V2 net-next 5/6] macvlan/macvtap: Add support for SCTP checksum offload.
From: Michael S. Tsirkin @ 2018-05-02 14:17 UTC (permalink / raw)
  To: Vlad Yasevich
  Cc: virtio-dev, marcelo.leitner, nhorman, netdev, virtualization,
	linux-sctp, Vladislav Yasevich
In-Reply-To: <cd94e936-f24c-de0d-7254-069054e33268@redhat.com>

On Wed, May 02, 2018 at 10:00:14AM -0400, Vlad Yasevich wrote:
> On 05/02/2018 09:46 AM, Michael S. Tsirkin wrote:
> > On Wed, May 02, 2018 at 09:27:00AM -0400, Vlad Yasevich wrote:
> >> On 05/01/2018 11:24 PM, Michael S. Tsirkin wrote:
> >>> On Tue, May 01, 2018 at 10:07:38PM -0400, Vladislav Yasevich wrote:
> >>>> Since we now have support for software CRC32c offload, turn it on
> >>>> for macvlan and macvtap devices so that guests can take advantage
> >>>> of offload SCTP checksums to the host or host hardware.
> >>>>
> >>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> >>>> ---
> >>>>  drivers/net/macvlan.c | 5 +++--
> >>>>  drivers/net/tap.c     | 8 +++++---
> >>>>  2 files changed, 8 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> >>>> index 725f4b4..646b730 100644
> >>>> --- a/drivers/net/macvlan.c
> >>>> +++ b/drivers/net/macvlan.c
> >>>> @@ -834,7 +834,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
> >>>>  
> >>>>  #define ALWAYS_ON_OFFLOADS \
> >>>>  	(NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \
> >>>> -	 NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL)
> >>>> +	 NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL | NETIF_F_SCTP_CRC)
> >>>>  
> >>>>  #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX)
> >>>>  
> >>>> @@ -842,7 +842,8 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
> >>>>  	(NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
> >>>>  	 NETIF_F_GSO | NETIF_F_TSO | NETIF_F_LRO | \
> >>>>  	 NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \
> >>>> -	 NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER)
> >>>> +	 NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER | \
> >>>> +	 NETIF_F_SCTP_CRC)
> >>>>  
> >>>>  #define MACVLAN_STATE_MASK \
> >>>>  	((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))
> >>>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> >>>> index 9b6cb78..2c8512b 100644
> >>>> --- a/drivers/net/tap.c
> >>>> +++ b/drivers/net/tap.c
> >>>> @@ -369,8 +369,7 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb)
> >>>>  		 *	  check, we either support them all or none.
> >>>>  		 */
> >>>>  		if (skb->ip_summed == CHECKSUM_PARTIAL &&
> >>>> -		    !(features & NETIF_F_CSUM_MASK) &&
> >>>> -		    skb_checksum_help(skb))
> >>>> +		    skb_csum_hwoffload_help(skb, features))
> >>>>  			goto drop;
> >>>>  		if (ptr_ring_produce(&q->ring, skb))
> >>>>  			goto drop;
> >>>> @@ -945,6 +944,9 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
> >>>>  		}
> >>>>  	}
> >>>>  
> >>>> +	if (arg & TUN_F_SCTP_CSUM)
> >>>> +		feature_mask |= NETIF_F_SCTP_CRC;
> >>>> +
> >>>
> >>> so this still affects TX, shouldn't this affect RX instead?
> >>
> >> There is no bit to set on the RX path just like there is no bit to set on the RX patch
> >> for TUN_F_CSUM.
> >>
> >> We only invert TSO offloads, not checksum offloads as the comment below states.
> >> For checksum,  macvtap has to compute the checksum itself in tap_handle_frame() above.
> >> It uses tx feature bits to see if needs do to the checksum.
> >>
> >> If you think we need another flag to macvtap to control RXCSUM, that would need to be
> >> separate and cover standard TCP checksum as well.
> >>
> >> -vlad
> > 
> > Confused. What is the meaning of TUN_F_SCTP_CSUM? I assume this is
> > a way for userspace to tell tun device: "I can handle
> > packets without SCTP checksum, pls send them my way".
> 
> Yes,  just as TUN_F_CSUM means that tun device can handle packets with
> partial tcp/udp checksum.
> 
> > 
> > Now what is the implication for macvtap? 
> 
> The implication is exactly the same as for TUN_F_CSUM.  If the
> flag is set on the macvtap device, the TX checksum feature is
> turned on.

I guess I will have to go back and re-read that code - I do not remember
what does TUN_F_CSUM does by now.  Here is a quick question that might
help me:

Let's assume userspace does not set TUN_F_SCTP_CSUM and device
does not calculate the checksums either. I would expect that with
macvtap this then behaves exactly like now with no overhead.

> 
> > And why  are
> > you setting NETIF_F_SCTP_CRC which is a flag
> > that affects packets sent by guest to host?
> 
> Mainly its because we are using just 1 flag to control checksum
> offloading and we need to be able control both tx and rx paths.

Well that's not really the case I think. What we have is controls for tx
offloads for tun. That's TUN_F_CSUM.
there are no rx offloads - userspace can send what it wants.

These are supposed to translate to rx offloads for macvtap.
tx offloads shouldn't be affected at all.

Maybe that's not the case - as I said need to go back and check.
Will try to find the time in the next couple of days.

> What you are suggesting that we either invert what TUN_F_CSUM
> is doing in macvtap case, or have another flag that lets us control
> TX and RX paths separately.
> 
> Either case, that would be separate work.
> -vlad

So assuming TUN_F_CSUM affects tx for macvtap do you
agree it's a bug? And should we add to it with
TUN_F_SCTP_CSUM doing the same?

> > 
> > 
> >>>
> >>>
> >>>>  	/* tun/tap driver inverts the usage for TSO offloads, where
> >>>>  	 * setting the TSO bit means that the userspace wants to
> >>>>  	 * accept TSO frames and turning it off means that user space
> >>>> @@ -1077,7 +1079,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
> >>>>  	case TUNSETOFFLOAD:
> >>>>  		/* let the user check for future flags */
> >>>>  		if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> >>>> -			    TUN_F_TSO_ECN | TUN_F_UFO))
> >>>> +			    TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_SCTP_CSUM))
> >>>>  			return -EINVAL;
> >>>>  
> >>>>  		rtnl_lock();
> >>>> -- 
> >>>> 2.9.5

^ permalink raw reply

* [PATCH V2 4/8] ARM: dts: stm32: Add syscfg on stm32mp1
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, andrew, christophe.roullier, linux-arm-kernel, netdev
In-Reply-To: <1525270723-18241-1-git-send-email-christophe.roullier@st.com>

System configuration controller is mainly used to manage
the compensation cell and other IOs and system related
settings.

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index bc3eddc..86421ba 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -167,6 +167,11 @@
 			#reset-cells = <1>;
 		};
 
+		syscfg: system-config@50020000 {
+			compatible = "syscon";
+			reg = <0x50020000 0x400>;
+		};
+
 		usart1: serial@5c000000 {
 			compatible = "st,stm32h7-uart";
 			reg = <0x5c000000 0x400>;
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH net-next] net/mlx4_en: optimizes get_fixed_ipv6_csum()
From: Tariq Toukan @ 2018-05-02 14:18 UTC (permalink / raw)
  To: Saeed Mahameed, davem@davemloft.net, edumazet@google.com
  Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com
In-Reply-To: <1524783416.1731.1.camel@mellanox.com>



On 27/04/2018 1:56 AM, Saeed Mahameed wrote:
> On Thu, 2018-04-19 at 08:49 -0700, Eric Dumazet wrote:
>> While trying to support CHECKSUM_COMPLETE for IPV6 fragments,
>> I had to experiments various hacks in get_fixed_ipv6_csum().
>> I must admit I could not find how to implement this :/
>>
>> However, get_fixed_ipv6_csum() does a lot of redundant operations,
>> calling csum_partial() twice.
>>
>> First csum_partial() computes the checksum of saddr and daddr,
>> put in @csum_pseudo_hdr. Undone later in the second csum_partial()
>> computed on whole ipv6 header.
>>
>> Then nexthdr is added once, added a second time, then substracted.
>>
>> payload_len is added once, then substracted.
>>
>> Really all this can be reduced to two add_csum(), to add back 6 bytes
>> that were removed by mlx4 when providing hw_checksum in RX
>> descriptor.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: Saeed Mahameed <saeedm@mellanox.com>
>> Cc: Tariq Toukan <tariqt@mellanox.com>
>> ---
>> Note: This patch, like other mlx4 patches can definitely wait
>> Tariq approval, thanks !
>>
> 
> LGTM,
> 
> Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
> 

Acked-by: Tariq Toukan <tariqt@mellanox.com>

Thanks Eric.

>>   drivers/net/ethernet/mellanox/mlx4/en_rx.c | 21 ++++++++----------
>> ---
>>   1 file changed, 8 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> index
>> 5c613c6663da51a4ae792eeb4d8956b54655786b..38c56fb6e5f5970f245dd56c38e
>> 1fc63a9349a07 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> @@ -593,30 +593,25 @@ static int get_fixed_ipv4_csum(__wsum
>> hw_checksum, struct sk_buff *skb,
>>   }
>>   
>>   #if IS_ENABLED(CONFIG_IPV6)
>> -/* In IPv6 packets, besides subtracting the pseudo header checksum,
>> - * we also compute/add the IP header checksum which
>> - * is not added by the HW.
>> +/* In IPv6 packets, hw_checksum lacks 6 bytes from IPv6 header:
>> + * 4 first bytes : priority, version, flow_lbl
>> + * and 2 additional bytes : nexthdr, hop_limit.
>>    */
>>   static int get_fixed_ipv6_csum(__wsum hw_checksum, struct sk_buff
>> *skb,
>>   			       struct ipv6hdr *ipv6h)
>>   {
>>   	__u8 nexthdr = ipv6h->nexthdr;
>> -	__wsum csum_pseudo_hdr = 0;
>> +	__wsum temp;
>>   
>>   	if (unlikely(nexthdr == IPPROTO_FRAGMENT ||
>>   		     nexthdr == IPPROTO_HOPOPTS ||
>>   		     nexthdr == IPPROTO_SCTP))
>>   		return -1;
>> -	hw_checksum = csum_add(hw_checksum, (__force
>> __wsum)htons(nexthdr));
>>   
>> -	csum_pseudo_hdr = csum_partial(&ipv6h->saddr,
>> -				       sizeof(ipv6h->saddr) +
>> sizeof(ipv6h->daddr), 0);
>> -	csum_pseudo_hdr = csum_add(csum_pseudo_hdr, (__force
>> __wsum)ipv6h->payload_len);
>> -	csum_pseudo_hdr = csum_add(csum_pseudo_hdr,
>> -				   (__force __wsum)htons(nexthdr));
>> -
>> -	skb->csum = csum_sub(hw_checksum, csum_pseudo_hdr);
>> -	skb->csum = csum_add(skb->csum, csum_partial(ipv6h,
>> sizeof(struct ipv6hdr), 0));
>> +	/* priority, version, flow_lbl */
>> +	temp = csum_add(hw_checksum, *(__wsum *)ipv6h);
>> +	/* nexthdr and hop_limit */
>> +	skb->csum = csum_add(temp, (__force __wsum)*(__be16
>> *)&ipv6h->nexthdr);
>>   	return 0;
>>   }
>>   #endif

^ permalink raw reply

* [PATCH] Revert "vhost: make msg padding explicit"
From: Michael S. Tsirkin @ 2018-05-02 14:19 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jason Wang, kvm, virtualization, netdev

This reverts commit 93c0d549c4c5a7382ad70de6b86610b7aae57406.

Unfortunately the padding will break 32 bit userspace.
Ouch. Need to add some compat code, revert for now.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/uapi/linux/vhost.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 5a8ad06..c51f8e5 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -68,7 +68,6 @@ struct vhost_iotlb_msg {
 
 struct vhost_msg {
 	int type;
-	int padding0;
 	union {
 		struct vhost_iotlb_msg iotlb;
 		__u8 padding[64];
-- 
MST

^ permalink raw reply related

* [PATCH V2 0/8] net: ethernet: stmmac: add support for stm32mp1
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, christophe.roullier, andrew

Patches to have Ethernet support on stm32mp1
Changelog:
Remark from Andrew Lunn
In drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
Add support of PHY_INTERFACE_MODE_RGMII_ID, PHY_INTERFACE_MODE_RGMII_RXID, 
PHY_INTERFACE_MODE_RGMII_TXID.

Remark from Rob Herring
In arch/arm/boot/dts/stm32mp157-pinctrl.dtsi:
Replace @0 with -0
In Documentation/devicetree/bindings/arm/stm32.txt
 .../devicetree/bindings/net/stm32-dwmac.txt
Update with requirement of Rob
In arch/arm/boot/dts/stm32mp157c.dtsi:
Remove compatible "st,stm32-syscfg" and use only generic "syscon"

Christophe Roullier (8):
  net: ethernet: stmmac: add adaptation for stm32mp157c.
  dt-bindings: stm32-dwmac: add support of MPU families
  ARM: dts: stm32: add ethernet pins to stm32mp157c
  ARM: dts: stm32: Add syscfg on stm32mp1
  ARM: dts: stm32: Add ethernet dwmac on stm32mp1
  net: stmmac: add dwmac-4.20a compatible
  ARM: dts: stm32: add support of ethernet on stm32mp157c-ev1
  dt-bindings: stm32: add compatible for syscon

 Documentation/devicetree/bindings/arm/stm32.txt    |   4 +
 .../devicetree/bindings/net/stm32-dwmac.txt        |  18 +-
 arch/arm/boot/dts/stm32mp157-pinctrl.dtsi          |  46 ++++
 arch/arm/boot/dts/stm32mp157c-ev1.dts              |  20 ++
 arch/arm/boot/dts/stm32mp157c.dtsi                 |  35 +++
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 270 +++++++++++++++++++--
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   3 +-
 7 files changed, 378 insertions(+), 18 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH V2 8/8] dt-bindings: stm32: add compatible for syscon
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, christophe.roullier, andrew
In-Reply-To: <1525270723-18241-1-git-send-email-christophe.roullier@st.com>

This patch describes syscon DT bindings.

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 Documentation/devicetree/bindings/arm/stm32.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/stm32.txt b/Documentation/devicetree/bindings/arm/stm32.txt
index 6808ed9..06e3834 100644
--- a/Documentation/devicetree/bindings/arm/stm32.txt
+++ b/Documentation/devicetree/bindings/arm/stm32.txt
@@ -8,3 +8,7 @@ using one of the following compatible strings:
   st,stm32f746
   st,stm32h743
   st,stm32mp157
+
+Required nodes:
+- syscon: the soc bus node must have a system controller node pointing to the
+  global control registers, with the compatible string "syscon";
-- 
1.9.1

^ permalink raw reply related

* [PATCH V2 2/8] dt-bindings: stm32-dwmac: add support of MPU families
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, christophe.roullier, andrew
In-Reply-To: <1525270723-18241-1-git-send-email-christophe.roullier@st.com>

Add description for Ethernet MPU families fields

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 Documentation/devicetree/bindings/net/stm32-dwmac.txt | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
index 489dbcb..1341012 100644
--- a/Documentation/devicetree/bindings/net/stm32-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
@@ -6,14 +6,28 @@ Please see stmmac.txt for the other unchanged properties.
 The device node has following properties.
 
 Required properties:
-- compatible:  Should be "st,stm32-dwmac" to select glue, and
+- compatible:  For MCU family should be "st,stm32-dwmac" to select glue, and
 	       "snps,dwmac-3.50a" to select IP version.
+	       For MPU family should be "st,stm32mp1-dwmac" to select
+	       glue, and "snps,dwmac-4.20a" to select IP version.
 - clocks: Must contain a phandle for each entry in clock-names.
 - clock-names: Should be "stmmaceth" for the host clock.
 	       Should be "mac-clk-tx" for the MAC TX clock.
 	       Should be "mac-clk-rx" for the MAC RX clock.
+	       For MPU family need to add also "ethstp" for power mode clock and,
+	                                       "syscfg-clk" for SYSCFG clock.
+- interrupt-names: Should contain a list of interrupt names corresponding to
+           the interrupts in the interrupts property, if available.
+		   Should be "macirq" for the main MAC IRQ
+		   Should be "eth_wake_irq" for the IT which wake up system
 - st,syscon : Should be phandle/offset pair. The phandle to the syscon node which
-	      encompases the glue register, and the offset of the control register.
+	       encompases the glue register, and the offset of the control register.
+
+Optional properties:
+- clock-names:     For MPU family "mac-clk-ck" for PHY without quartz
+- st,int-phyclk (boolean) :  valid only where PHY do not have quartz and need to be clock
+	           by RCC
+
 Example:
 
 	ethernet@40028000 {
-- 
1.9.1

^ permalink raw reply related

* [PATCH V2 5/8] ARM: dts: stm32: Add ethernet dwmac on stm32mp1
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, christophe.roullier, andrew
In-Reply-To: <1525270723-18241-1-git-send-email-christophe.roullier@st.com>

Add Ethernet support (Synopsys MAC IP 4.20a) on stm32mp1 SOC.
Enable feature supported by the stmmac driver, such as TSO.

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 86421ba..79def3b 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -179,5 +179,35 @@
 			clocks = <&rcc USART1_K>;
 			status = "disabled";
 		};
+
+		stmmac_axi_config_0: stmmac-axi-config {
+			snps,wr_osr_lmt = <0x7>;
+			snps,rd_osr_lmt = <0x7>;
+			snps,blen = <0 0 0 0 16 8 4>;
+		};
+
+		ethernet0: ethernet@5800a000 {
+			compatible = "st,stm32mp1-dwmac", "snps,dwmac-4.20a";
+			reg = <0x5800a000 0x2000>;
+			reg-names = "stmmaceth";
+			interrupts-extended = <&intc GIC_SPI 61 IRQ_TYPE_NONE>;
+			interrupt-names = "macirq";
+			clock-names = "stmmaceth",
+				      "mac-clk-tx",
+				      "mac-clk-rx",
+				      "ethstp",
+				      "syscfg-clk";
+			clocks = <&rcc ETHMAC>,
+				 <&rcc ETHTX>,
+				 <&rcc ETHRX>,
+				 <&rcc ETHSTP>,
+				 <&rcc SYSCFG>;
+			st,syscon = <&syscfg 0x4>;
+			snps,mixed-burst;
+			snps,pbl = <2>;
+			snps,axi-config = <&stmmac_axi_config_0>;
+			snps,tso;
+			status = "disabled";
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related

* [PATCH V2 6/8] net: stmmac: add dwmac-4.20a compatible
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, christophe.roullier, andrew
In-Reply-To: <1525270723-18241-1-git-send-email-christophe.roullier@st.com>

Manage dwmac-4.20a version from synopsys

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index ebd3e5f..6d141f3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -472,7 +472,8 @@ struct plat_stmmacenet_data *
 	}
 
 	if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
-	    of_device_is_compatible(np, "snps,dwmac-4.10a")) {
+	    of_device_is_compatible(np, "snps,dwmac-4.10a") ||
+	    of_device_is_compatible(np, "snps,dwmac-4.20a")) {
 		plat->has_gmac4 = 1;
 		plat->has_gmac = 0;
 		plat->pmt = 1;
-- 
1.9.1

^ permalink raw reply related

* [PATCH V2 1/8] net: ethernet: stmmac: add adaptation for stm32mp157c.
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, christophe.roullier, andrew
In-Reply-To: <1525270723-18241-1-git-send-email-christophe.roullier@st.com>

Glue codes to support stm32mp157c device and stay
compatible with stm32 mcu family

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 270 ++++++++++++++++++++--
 1 file changed, 255 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 9e6db16..f51e327 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -16,49 +16,183 @@
 #include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/stmmac.h>
 
 #include "stmmac_platform.h"
 
-#define MII_PHY_SEL_MASK	BIT(23)
+#define SYSCFG_MCU_ETH_MASK		BIT(23)
+#define SYSCFG_MP1_ETH_MASK		GENMASK(23, 16)
+
+#define SYSCFG_PMCR_ETH_CLK_SEL		BIT(16)
+#define SYSCFG_PMCR_ETH_REF_CLK_SEL	BIT(17)
+#define SYSCFG_PMCR_ETH_SEL_MII		BIT(20)
+#define SYSCFG_PMCR_ETH_SEL_RGMII	BIT(21)
+#define SYSCFG_PMCR_ETH_SEL_RMII	BIT(23)
+#define SYSCFG_PMCR_ETH_SEL_GMII	0
+#define SYSCFG_MCU_ETH_SEL_MII		0
+#define SYSCFG_MCU_ETH_SEL_RMII		1
 
 struct stm32_dwmac {
 	struct clk *clk_tx;
 	struct clk *clk_rx;
+	struct clk *clk_eth_ck;
+	struct clk *clk_ethstp;
+	struct clk *syscfg_clk;
+	bool int_phyclk;	/* Clock from RCC to drive PHY */
 	u32 mode_reg;		/* MAC glue-logic mode register */
 	struct regmap *regmap;
 	u32 speed;
+	const struct stm32_ops *ops;
+	struct device *dev;
+};
+
+struct stm32_ops {
+	int (*set_mode)(struct plat_stmmacenet_data *plat_dat);
+	int (*clk_prepare)(struct stm32_dwmac *dwmac, bool prepare);
+	int (*suspend)(struct stm32_dwmac *dwmac);
+	void (*resume)(struct stm32_dwmac *dwmac);
+	int (*parse_data)(struct stm32_dwmac *dwmac,
+			  struct device *dev);
+	u32 syscfg_eth_mask;
 };
 
 static int stm32_dwmac_init(struct plat_stmmacenet_data *plat_dat)
 {
 	struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
-	u32 reg = dwmac->mode_reg;
-	u32 val;
 	int ret;
 
-	val = (plat_dat->interface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
-	ret = regmap_update_bits(dwmac->regmap, reg, MII_PHY_SEL_MASK, val);
-	if (ret)
-		return ret;
+	if (dwmac->ops->set_mode) {
+		ret = dwmac->ops->set_mode(plat_dat);
+		if (ret)
+			return ret;
+	}
 
 	ret = clk_prepare_enable(dwmac->clk_tx);
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(dwmac->clk_rx);
-	if (ret)
-		clk_disable_unprepare(dwmac->clk_tx);
+	if (!dwmac->dev->power.is_suspended) {
+		ret = clk_prepare_enable(dwmac->clk_rx);
+		if (ret) {
+			clk_disable_unprepare(dwmac->clk_tx);
+			return ret;
+		}
+	}
+
+	if (dwmac->ops->clk_prepare) {
+		ret = dwmac->ops->clk_prepare(dwmac, true);
+		if (ret) {
+			clk_disable_unprepare(dwmac->clk_rx);
+			clk_disable_unprepare(dwmac->clk_tx);
+		}
+	}
 
 	return ret;
 }
 
+static int stm32mp1_clk_prepare(struct stm32_dwmac *dwmac, bool prepare)
+{
+	int ret = 0;
+
+	if (prepare) {
+		ret = clk_prepare_enable(dwmac->syscfg_clk);
+		if (ret)
+			return ret;
+
+		if (dwmac->int_phyclk) {
+			ret = clk_prepare_enable(dwmac->clk_eth_ck);
+			if (ret) {
+				clk_disable_unprepare(dwmac->syscfg_clk);
+				return ret;
+			}
+		}
+	} else {
+		clk_disable_unprepare(dwmac->syscfg_clk);
+		if (dwmac->int_phyclk)
+			clk_disable_unprepare(dwmac->clk_eth_ck);
+	}
+	return ret;
+}
+
+static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
+{
+	struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
+	u32 reg = dwmac->mode_reg;
+	int val;
+
+	switch (plat_dat->interface) {
+	case PHY_INTERFACE_MODE_MII:
+		val = SYSCFG_PMCR_ETH_SEL_MII;
+		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
+		break;
+	case PHY_INTERFACE_MODE_GMII:
+		val = SYSCFG_PMCR_ETH_SEL_GMII;
+		if (dwmac->int_phyclk)
+			val |= SYSCFG_PMCR_ETH_CLK_SEL;
+		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n");
+		break;
+	case PHY_INTERFACE_MODE_RMII:
+		val = SYSCFG_PMCR_ETH_SEL_RMII;
+		if (dwmac->int_phyclk)
+			val |= SYSCFG_PMCR_ETH_REF_CLK_SEL;
+		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RMII\n");
+		break;
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		val = SYSCFG_PMCR_ETH_SEL_RGMII;
+		if (dwmac->int_phyclk)
+			val |= SYSCFG_PMCR_ETH_CLK_SEL;
+		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RGMII\n");
+		break;
+	default:
+		pr_debug("SYSCFG init :  Do not manage %d interface\n",
+			 plat_dat->interface);
+		/* Do not manage others interfaces */
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(dwmac->regmap, reg,
+				 dwmac->ops->syscfg_eth_mask, val);
+}
+
+static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat)
+{
+	struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
+	u32 reg = dwmac->mode_reg;
+	int val;
+
+	switch (plat_dat->interface) {
+	case PHY_INTERFACE_MODE_MII:
+		val = SYSCFG_MCU_ETH_SEL_MII;
+		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
+		break;
+	case PHY_INTERFACE_MODE_RMII:
+		val = SYSCFG_MCU_ETH_SEL_RMII;
+		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RMII\n");
+		break;
+	default:
+		pr_debug("SYSCFG init :  Do not manage %d interface\n",
+			 plat_dat->interface);
+		/* Do not manage others interfaces */
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(dwmac->regmap, reg,
+				 dwmac->ops->syscfg_eth_mask, val);
+}
+
 static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac)
 {
 	clk_disable_unprepare(dwmac->clk_tx);
 	clk_disable_unprepare(dwmac->clk_rx);
+
+	if (dwmac->ops->clk_prepare)
+		dwmac->ops->clk_prepare(dwmac, false);
 }
 
 static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
@@ -70,15 +204,22 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
 	/*  Get TX/RX clocks */
 	dwmac->clk_tx = devm_clk_get(dev, "mac-clk-tx");
 	if (IS_ERR(dwmac->clk_tx)) {
-		dev_err(dev, "No tx clock provided...\n");
+		dev_err(dev, "No ETH Tx clock provided...\n");
 		return PTR_ERR(dwmac->clk_tx);
 	}
+
 	dwmac->clk_rx = devm_clk_get(dev, "mac-clk-rx");
 	if (IS_ERR(dwmac->clk_rx)) {
-		dev_err(dev, "No rx clock provided...\n");
+		dev_err(dev, "No ETH Rx clock provided...\n");
 		return PTR_ERR(dwmac->clk_rx);
 	}
 
+	if (dwmac->ops->parse_data) {
+		err = dwmac->ops->parse_data(dwmac, dev);
+		if (err)
+			return err;
+	}
+
 	/* Get mode register */
 	dwmac->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
 	if (IS_ERR(dwmac->regmap))
@@ -91,11 +232,46 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
 	return err;
 }
 
+static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
+			       struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+
+	dwmac->int_phyclk = of_property_read_bool(np, "st,int-phyclk");
+
+	/* Check if internal clk from RCC selected */
+	if (dwmac->int_phyclk) {
+		/*  Get ETH_CLK clocks */
+		dwmac->clk_eth_ck = devm_clk_get(dev, "eth-ck");
+		if (IS_ERR(dwmac->clk_eth_ck)) {
+			dev_err(dev, "No ETH CK clock provided...\n");
+			return PTR_ERR(dwmac->clk_eth_ck);
+		}
+	}
+
+	/*  Clock used for low power mode */
+	dwmac->clk_ethstp = devm_clk_get(dev, "ethstp");
+	if (IS_ERR(dwmac->clk_ethstp)) {
+		dev_err(dev, "No ETH peripheral clock provided for CStop mode ...\n");
+		return PTR_ERR(dwmac->clk_ethstp);
+	}
+
+	/*  Clock for sysconfig */
+	dwmac->syscfg_clk = devm_clk_get(dev, "syscfg-clk");
+	if (IS_ERR(dwmac->syscfg_clk)) {
+		dev_err(dev, "No syscfg clock provided...\n");
+		return PTR_ERR(dwmac->syscfg_clk);
+	}
+
+	return 0;
+}
+
 static int stm32_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;
 	struct stmmac_resources stmmac_res;
 	struct stm32_dwmac *dwmac;
+	const struct stm32_ops *data;
 	int ret;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
@@ -112,6 +288,16 @@ static int stm32_dwmac_probe(struct platform_device *pdev)
 		goto err_remove_config_dt;
 	}
 
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data) {
+		dev_err(&pdev->dev, "no of match data provided\n");
+		ret = -EINVAL;
+		goto err_remove_config_dt;
+	}
+
+	dwmac->ops = data;
+	dwmac->dev = &pdev->dev;
+
 	ret = stm32_dwmac_parse_data(dwmac, &pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Unable to parse OF data\n");
@@ -149,15 +335,48 @@ static int stm32_dwmac_remove(struct platform_device *pdev)
 	return ret;
 }
 
+static int stm32mp1_suspend(struct stm32_dwmac *dwmac)
+{
+	int ret = 0;
+
+	ret = clk_prepare_enable(dwmac->clk_ethstp);
+	if (ret)
+		return ret;
+
+	clk_disable_unprepare(dwmac->clk_tx);
+	clk_disable_unprepare(dwmac->syscfg_clk);
+	if (dwmac->int_phyclk)
+		clk_disable_unprepare(dwmac->clk_eth_ck);
+
+	return ret;
+}
+
+static void stm32mp1_resume(struct stm32_dwmac *dwmac)
+{
+	clk_disable_unprepare(dwmac->clk_ethstp);
+}
+
+static int stm32mcu_suspend(struct stm32_dwmac *dwmac)
+{
+	clk_disable_unprepare(dwmac->clk_tx);
+	clk_disable_unprepare(dwmac->clk_rx);
+
+	return 0;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int stm32_dwmac_suspend(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct stm32_dwmac *dwmac = priv->plat->bsp_priv;
+
 	int ret;
 
 	ret = stmmac_suspend(dev);
-	stm32_dwmac_clk_disable(priv->plat->bsp_priv);
+
+	if (dwmac->ops->suspend)
+		ret = dwmac->ops->suspend(dwmac);
 
 	return ret;
 }
@@ -166,8 +385,12 @@ static int stm32_dwmac_resume(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct stm32_dwmac *dwmac = priv->plat->bsp_priv;
 	int ret;
 
+	if (dwmac->ops->resume)
+		dwmac->ops->resume(dwmac);
+
 	ret = stm32_dwmac_init(priv->plat);
 	if (ret)
 		return ret;
@@ -181,8 +404,24 @@ static int stm32_dwmac_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(stm32_dwmac_pm_ops,
 	stm32_dwmac_suspend, stm32_dwmac_resume);
 
+static struct stm32_ops stm32mcu_dwmac_data = {
+	.set_mode = stm32mcu_set_mode,
+	.suspend = stm32mcu_suspend,
+	.syscfg_eth_mask = SYSCFG_MCU_ETH_MASK
+};
+
+static struct stm32_ops stm32mp1_dwmac_data = {
+	.set_mode = stm32mp1_set_mode,
+	.clk_prepare = stm32mp1_clk_prepare,
+	.suspend = stm32mp1_suspend,
+	.resume = stm32mp1_resume,
+	.parse_data = stm32mp1_parse_data,
+	.syscfg_eth_mask = SYSCFG_MP1_ETH_MASK
+};
+
 static const struct of_device_id stm32_dwmac_match[] = {
-	{ .compatible = "st,stm32-dwmac"},
+	{ .compatible = "st,stm32-dwmac", .data = &stm32mcu_dwmac_data},
+	{ .compatible = "st,stm32mp1-dwmac", .data = &stm32mp1_dwmac_data},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
@@ -199,5 +438,6 @@ static SIMPLE_DEV_PM_OPS(stm32_dwmac_pm_ops,
 module_platform_driver(stm32_dwmac_driver);
 
 MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
-MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
+MODULE_AUTHOR("Christophe Roullier <christophe.roullier@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 DWMAC Specific Glue layer");
 MODULE_LICENSE("GPL v2");
-- 
1.9.1

^ permalink raw reply related

* [PATCH V2 7/8] ARM: dts: stm32: add support of ethernet on stm32mp157c-ev1
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, christophe.roullier, andrew
In-Reply-To: <1525270723-18241-1-git-send-email-christophe.roullier@st.com>

MAC is connected to a PHY in RGMII mode.

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 arch/arm/boot/dts/stm32mp157c-ev1.dts | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts
index 57e6dbc..a7fee5c 100644
--- a/arch/arm/boot/dts/stm32mp157c-ev1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts
@@ -17,5 +17,25 @@
 
 	aliases {
 		serial0 = &uart4;
+		ethernet0 = &ethernet0;
+	};
+};
+
+&ethernet0 {
+	status = "okay";
+	pinctrl-0 = <&ethernet0_rgmii_pins_a>;
+	pinctrl-1 = <&ethernet0_rgmii_pins_sleep_a>;
+	pinctrl-names = "default", "sleep";
+	phy-mode = "rgmii";
+	max-speed = <1000>;
+	phy-handle = <&phy0>;
+
+	mdio0 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+		phy0: ethernet-phy@0 {
+			reg = <0>;
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related

* [PATCH V2 3/8] ARM: dts: stm32: add ethernet pins to stm32mp157c
From: Christophe Roullier @ 2018-05-02 14:18 UTC (permalink / raw)
  To: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, christophe.roullier, andrew
In-Reply-To: <1525270723-18241-1-git-send-email-christophe.roullier@st.com>

Add ethernet pins on stm32mp157c.

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
 arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 46 +++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
index 6f044100..cf83eb244 100644
--- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
@@ -158,6 +158,52 @@
 					bias-disable;
 				};
 			};
+
+			ethernet0_rgmii_pins_a: rgmii-0 {
+				pins1 {
+					pinmux = <STM32_PINMUX('G', 5, AF11)>, /* ETH_RGMII_CLK125 */
+						 <STM32_PINMUX('G', 4, AF11)>, /* ETH_RGMII_GTX_CLK */
+						 <STM32_PINMUX('G', 13, AF11)>, /* ETH_RGMII_TXD0 */
+						 <STM32_PINMUX('G', 14, AF11)>, /* ETH_RGMII_TXD1 */
+						 <STM32_PINMUX('C', 2, AF11)>, /* ETH_RGMII_TXD2 */
+						 <STM32_PINMUX('E', 2, AF11)>, /* ETH_RGMII_TXD3 */
+						 <STM32_PINMUX('B', 11, AF11)>, /* ETH_RGMII_TX_CTL */
+						 <STM32_PINMUX('A', 2, AF11)>, /* ETH_MDIO */
+						 <STM32_PINMUX('C', 1, AF11)>; /* ETH_MDC */
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <3>;
+				};
+				pins2 {
+					pinmux = <STM32_PINMUX('C', 4, AF11)>, /* ETH_RGMII_RXD0 */
+						 <STM32_PINMUX('C', 5, AF11)>, /* ETH_RGMII_RXD1 */
+						 <STM32_PINMUX('B', 0, AF11)>, /* ETH_RGMII_RXD2 */
+						 <STM32_PINMUX('B', 1, AF11)>, /* ETH_RGMII_RXD3 */
+						 <STM32_PINMUX('A', 1, AF11)>, /* ETH_RGMII_RX_CLK */
+						 <STM32_PINMUX('A', 7, AF11)>; /* ETH_RGMII_RX_CTL */
+					bias-disable;
+				};
+			};
+
+			ethernet0_rgmii_pins_sleep_a: rgmii-sleep-0 {
+				pins1 {
+					pinmux = <STM32_PINMUX('G', 5, ANALOG)>, /* ETH_RGMII_CLK125 */
+						 <STM32_PINMUX('G', 4, ANALOG)>, /* ETH_RGMII_GTX_CLK */
+						 <STM32_PINMUX('G', 13, ANALOG)>, /* ETH_RGMII_TXD0 */
+						 <STM32_PINMUX('G', 14, ANALOG)>, /* ETH_RGMII_TXD1 */
+						 <STM32_PINMUX('C', 2, ANALOG)>, /* ETH_RGMII_TXD2 */
+						 <STM32_PINMUX('E', 2, ANALOG)>, /* ETH_RGMII_TXD3 */
+						 <STM32_PINMUX('B', 11, ANALOG)>, /* ETH_RGMII_TX_CTL */
+						 <STM32_PINMUX('A', 2, ANALOG)>, /* ETH_MDIO */
+						 <STM32_PINMUX('C', 1, ANALOG)>, /* ETH_MDC */
+						 <STM32_PINMUX('C', 4, ANALOG)>, /* ETH_RGMII_RXD0 */
+						 <STM32_PINMUX('C', 5, ANALOG)>, /* ETH_RGMII_RXD1 */
+						 <STM32_PINMUX('B', 0, ANALOG)>, /* ETH_RGMII_RXD2 */
+						 <STM32_PINMUX('B', 1, ANALOG)>, /* ETH_RGMII_RXD3 */
+						 <STM32_PINMUX('A', 1, ANALOG)>, /* ETH_RGMII_RX_CLK */
+						 <STM32_PINMUX('A', 7, ANALOG)>; /* ETH_RGMII_RX_CTL */
+				};
+			};
 		};
 
 		pinctrl_z: pin-controller-z {
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] vhost: make msg padding explicit
From: Michael S. Tsirkin @ 2018-05-02 14:19 UTC (permalink / raw)
  To: David Miller; +Cc: kevin, kvm, netdev, linux-kernel, virtualization
In-Reply-To: <20180502.100446.106883595468297057.davem@davemloft.net>

On Wed, May 02, 2018 at 10:04:46AM -0400, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Wed, 2 May 2018 16:36:37 +0300
> 
> > Ouch.  True - and in particular the 32 bit ABI on 64 bit kernels doesn't
> > work at all. Hmm. It's relatively new and maybe there aren't any 32 bit
> > users yet. Thoughts?
> 
> If it's been in a released kernel version, we really aren't at liberty
> to play "maybe nobody uses this" UAPI changing games.
> 
> Please send me a revert.

Sent. Looking at compat mess now.

-- 
MST

^ permalink raw reply

* Re: [PATCH] NET/netlink: optimize output of seq_puts in af_netlink.c
From: David Miller @ 2018-05-02 14:19 UTC (permalink / raw)
  To: tsu.yubo; +Cc: xiyou.wangcong, yuzibode, netdev, kernel-janitors
In-Reply-To: <20180502095421.GA15014@yubo-2>

From: Bo YU <tsu.yubo@gmail.com>
Date: Wed, 2 May 2018 05:54:24 -0400

> Optimization of command output: `cat /proc/net/netlink`
> 
> After the patch, we will get:
> 
> https://clbin.com/lnu4L
> 
> Signed-off-by: Bo YU <tsu.yubo@gmail.com>
> ---
>  net/netlink/af_netlink.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 55342c4d5cec..2e2dd88fc79f 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -2606,13 +2606,13 @@ static int netlink_seq_show(struct seq_file
> *seq, void *v)
>  {
>  	if (v == SEQ_START_TOKEN) {
>  		seq_puts(seq,
> -			 "sk       Eth Pid    Groups   "
> -			 "Rmem     Wmem     Dump     Locks     Drops     Inode\n");
> +			 "sk               Eth Pid        Groups   "
> + "Rmem Wmem Dump Locks Drops Inode\n");

Please do not break the indentation of the code like this.

I wish to unfortunately say, that generally speaking, your patch
submissions are not of the best quality, and take up a lot of reviewer
time and resources as a result.

If you do not improve the quality of your submissions, I am giving
you a kind warning that the amount of care and review your patches
will receive will become lower.  Your submissions might even get to
the point wheere they are effectively ignored.

So please put more care into your work.

Thank you.

^ permalink raw reply

* Re: arch/x86/net/bpf_jit_comp conflicts. was: [tip:x86/cleanups] x86/bpf: Clean up non-standard comments, to make the code more readable
From: Daniel Borkmann @ 2018-05-02 14:21 UTC (permalink / raw)
  To: Alexei Starovoitov, peterz, edumazet, davem, linux-kernel,
	torvalds, bp, hpa, mingo, tglx, linux-tip-commits, netdev
In-Reply-To: <7741d4cc-2d1d-9831-e0ce-38afdd41f0a1@fb.com>

On 04/28/2018 12:16 AM, Alexei Starovoitov wrote:
> On 4/27/18 5:13 AM, Daniel Borkmann wrote:
>> On 04/27/2018 01:00 PM, tip-bot for Ingo Molnar wrote:
>>> Commit-ID:  5f26c50143f58f256535bee8d93a105f36d4d2da
>>> Gitweb:     https://git.kernel.org/tip/5f26c50143f58f256535bee8d93a105f36d4d2da
>>> Author:     Ingo Molnar <mingo@kernel.org>
>>> AuthorDate: Fri, 27 Apr 2018 11:54:40 +0200
>>> Committer:  Ingo Molnar <mingo@kernel.org>
>>> CommitDate: Fri, 27 Apr 2018 12:42:04 +0200
>>>
>>> x86/bpf: Clean up non-standard comments, to make the code more readable
>>>
>>> So by chance I looked into x86 assembly in arch/x86/net/bpf_jit_comp.c and
>>> noticed the weird and inconsistent comment style it mistakenly learned from
>>> the networking code:
>>>
>>>  /* Multi-line comment ...
>>>   * ... looks like this.
>>>   */
>>>
[...]
>>> No change in functionality.
>>
>> Thanks for the cleanup, looks fine to me!
> 
> same here. thanks for the cleanup!
> 
>>> ( In case this commit causes conflicts with pending development code
>>>   I'll be glad to help resolve any conflicts! )
>>
>> Any objections if we would simply route this via bpf-next tree, otherwise
>> this will indeed cause really ugly merge conflicts throughout the JIT with
>> pending work.

Since no one hollered I've cherry picked this into bpf-next tree so that
upcoming BPF work can be rebased on top of this, thanks Ingo!

> right. would be much better to route this patch via bpf-next.
> Though all the changes are cleanups in comments I'm pretty sure
> they will conflict with other changes we're doing.
> 
> Ingo,
> could you please drop this patch from tip tree and resend it to us?
> I cannot find the original patch in any public mailing list.
> Only in tip-bot notification.
> 
> Personally I don't care whether bpf jit code uses networking
> or non-networking style of comments, but will be happy to enforce
> non-networking for this file in the future, since that seems to be the
> preference.
> 
> Thanks

^ permalink raw reply

* Re: [PATCH V2 net-next 1/6] virtio: Add support for SCTP checksum offloading
From: Michael S. Tsirkin @ 2018-05-02 14:21 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization,
	virtio-dev, jasowang, nhorman, Vladislav Yasevich
In-Reply-To: <20180502141413.GD5105@localhost.localdomain>

On Wed, May 02, 2018 at 11:14:13AM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, May 02, 2018 at 06:16:45AM +0300, Michael S. Tsirkin wrote:
> > On Tue, May 01, 2018 at 10:07:34PM -0400, Vladislav Yasevich wrote:
> > > To support SCTP checksum offloading, we need to add a new feature
> > > to virtio_net, so we can negotiate support between the hypervisor
> > > and the guest.
> > > The HOST feature bit signifies offloading support for transmit and
> > > enables device offload features.
> > > The GUEST feature bit signifies offloading support of recieve and
> > > is currently only used by the driver in case of xdp.
> > >
> > > That patch also adds an addition virtio_net header flag which
> > > mirrors the skb->csum_not_inet flag.  This flags is used to indicate
> > > that is this an SCTP packet that needs its checksum computed by the
> > > lower layer.  In this case, the lower layer is the host hypervisor or
> > > possibly HW nic that supporst CRC32c offload.
> > >
> > > In the case that GUEST feature bit is flag, it will be possible to
> > > receive a virtio_net header with this bit set, which will set the
> > > corresponding skb bit.  SCTP protocol will be updated to correctly
> > > deal with it.
> > >
> > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> > > ---
> > >  drivers/net/virtio_net.c        | 14 +++++++++++++-
> > >  include/linux/virtio_net.h      |  6 ++++++
> > >  include/uapi/linux/virtio_net.h |  5 +++++
> > >  3 files changed, 24 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 7b187ec..34af280 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -2148,6 +2148,8 @@ static int virtnet_clear_guest_offloads(struct virtnet_info *vi)
> > >
> > >  	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM))
> > >  		offloads = 1ULL << VIRTIO_NET_F_GUEST_CSUM;
> > > +	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_SCTP_CSUM))
> > > +		offloads |= 1ULL << VIRTIO_NET_F_GUEST_SCTP_CSUM;
> > >
> > >  	return virtnet_set_guest_offloads(vi, offloads);
> > >  }
> > > @@ -2160,6 +2162,8 @@ static int virtnet_restore_guest_offloads(struct virtnet_info *vi)
> > >  		return 0;
> > >  	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM))
> > >  		offloads |= 1ULL << VIRTIO_NET_F_GUEST_CSUM;
> > > +	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_SCTP_CSUM))
> > > +		offloads |= 1ULL << VIRTIO_NET_F_GUEST_SCTP_CSUM;
> > >
> > >  	return virtnet_set_guest_offloads(vi, offloads);
> > >  }
> > > @@ -2724,6 +2728,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> > >  	/* Do we support "hardware" checksums? */
> > >  	if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> > >  		/* This opens up the world of extra features. */
> > > +
> > >  		dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG;
> > >  		if (csum)
> > >  			dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG;
> > > @@ -2746,9 +2751,15 @@ static int virtnet_probe(struct virtio_device *vdev)
> > >  			dev->features |= dev->hw_features & NETIF_F_ALL_TSO;
> > >  		/* (!csum && gso) case will be fixed by register_netdev() */
> > >  	}
> > > +
> > >  	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
> > >  		dev->features |= NETIF_F_RXCSUM;
> > >
> > > +	if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_SCTP_CSUM)) {
> > > +		dev->hw_features |= NETIF_F_SCTP_CRC;
> > > +		dev->features |= NETIF_F_SCTP_CRC;
> > > +	}
> > > +
> > >  	dev->vlan_features = dev->features;
> > >
> > >  	/* MTU range: 68 - 65535 */
> > > @@ -2962,7 +2973,8 @@ static struct virtio_device_id id_table[] = {
> > >  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> > >  	VIRTIO_NET_F_CTRL_MAC_ADDR, \
> > >  	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> > > -	VIRTIO_NET_F_SPEED_DUPLEX
> > > +	VIRTIO_NET_F_SPEED_DUPLEX, \
> > > +	VIRTIO_NET_F_HOST_SCTP_CSUM, VIRTIO_NET_F_GUEST_SCTP_CSUM
> > >
> > >  static unsigned int features[] = {
> > >  	VIRTNET_FEATURES,
> > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > > index f144216..28fffdc 100644
> > > --- a/include/linux/virtio_net.h
> > > +++ b/include/linux/virtio_net.h
> > > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> > >
> > >  		if (!skb_partial_csum_set(skb, start, off))
> > >  			return -EINVAL;
> > > +
> > > +		if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET)
> > > +			skb->csum_not_inet = 1;
> > >  	}
> > >
> > >  	if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> > > @@ -91,6 +94,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> > >  				skb_checksum_start_offset(skb));
> > >  		hdr->csum_offset = __cpu_to_virtio16(little_endian,
> > >  				skb->csum_offset);
> > > +
> > > +		if (skb->csum_not_inet)
> > > +			hdr->flags |= VIRTIO_NET_HDR_F_CSUM_NOT_INET;
> > >  	} else if (has_data_valid &&
> > >  		   skb->ip_summed == CHECKSUM_UNNECESSARY) {
> > >  		hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
> > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > > index 5de6ed3..9dfca1a 100644
> > > --- a/include/uapi/linux/virtio_net.h
> > > +++ b/include/uapi/linux/virtio_net.h
> > > @@ -57,6 +57,10 @@
> > >  					 * Steering */
> > >  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
> > >
> > > +#define VIRTIO_NET_F_GUEST_SCTP_CSUM  61 /* Guest handles SCTP pks w/ partial
> > > +					  * csum */
> > > +#define VIRTIO_NET_F_HOST_SCTP_CSUM   62 /* HOST handles SCTP pkts w/ partial
> > > +					  * csum */
> > >  #define VIRTIO_NET_F_SPEED_DUPLEX 63	/* Device set linkspeed and duplex */
> > >
> > >  #ifndef VIRTIO_NET_NO_LEGACY
> > > @@ -101,6 +105,7 @@ struct virtio_net_config {
> > >  struct virtio_net_hdr_v1 {
> > >  #define VIRTIO_NET_HDR_F_NEEDS_CSUM	1	/* Use csum_start, csum_offset */
> > >  #define VIRTIO_NET_HDR_F_DATA_VALID	2	/* Csum is valid */
> > > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET  4       /* Checksum is not inet */
> >
> > Both comment and name are not very informative.
> > How about just saying CRC32c ?
> 
> csum_not_inet is following the nomenclature used in sk_buff. Initially
> Davide had named the sk_buff field after crc32c but then it was argued
> that maybe another check method may be introduced later and the name
> would be bogus, thus why the "csum_not_inet".

That's sk_buff internals, since Linux can change these
at any time without breaking anything.

virtio defines an ABI so we won't be able to make it
mean something else, need to document it fully.
"Go read linux net core code" is not a good answer
to the natural question "what does this bit in the
interface do".

> >
> > >  	__u8 flags;
> > >  #define VIRTIO_NET_HDR_GSO_NONE		0	/* Not a GSO frame */
> > >  #define VIRTIO_NET_HDR_GSO_TCPV4	1	/* GSO frame, IPv4 TCP (TSO) */
> > > --
> > > 2.9.5
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >

^ permalink raw reply

* Re: [PATCH] net: stmmac: Avoid VLA usage
From: Alexandre Torgue @ 2018-05-02 14:22 UTC (permalink / raw)
  To: Jose Abreu, Kees Cook; +Cc: Giuseppe Cavallaro, LKML, Network Development
In-Reply-To: <4c209618-8da0-ef1d-ff79-5da3f235b9bc@synopsys.com>



On 05/02/2018 04:07 PM, Jose Abreu wrote:
> 
> 
> On 02-05-2018 13:36, Kees Cook wrote:
>> On Wed, May 2, 2018 at 1:54 AM, Jose Abreu <Jose.Abreu@synopsys.com> wrote:
>>> Hi Kees,
>>>
>>> On 01-05-2018 22:01, Kees Cook wrote:
>>>> In the quest to remove all stack VLAs from the kernel[1], this switches
>>>> the "status" stack buffer to use the existing small (8) upper bound on
>>>> how many queues can be checked for DMA, and adds a sanity-check just to
>>>> make sure it doesn't operate under pathological conditions.
>>>>
>>>> [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__lkml.kernel.org_r_CA-2B55aFzCG-2DzNmZwX4A2FQpadafLfEzK6CC-3DqPXydAacU1RqZWA-40mail.gmail.com&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=TBD6a7UY2VbpPmV9LOW_eHAyg8uPq1ZPDhq93VROTVE&s=4fvOST1HhWmZ4lThQe-dHCJYEXNOwey00BCXOWm8tKo&e=
>>>>
>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>>
>>> I rather prefer the variables declaration in reverse-tree order,
>>> but thats just a minor pick.
>> I can explicitly reorder the other variables, if you want?
> 
> No need by me, unless Giuseppe or Alexandre prefer that. Thanks!

No need.

> 
> Best Regards,
> Jose Miguel Abreu
> 
>>
>>> Reviewed-by: Jose Abreu <joabreu@synopsys.com>
>> Thanks!
>>
>>> PS: Is VLA warning switch in gcc already active? Because I didn't
>>> see this warning in my builds.
>> It is not. A bunch of people have been building with KCFLAGS=-Wvla to
>> find the VLAs and sending patches. Once we get rid of them all, we can
>> add the flag to the top-level Makefile.
>>
>> -Kees
>>
> 

^ permalink raw reply

* Re: [PATCH V2 net-next 5/6] macvlan/macvtap: Add support for SCTP checksum offload.
From: Vlad Yasevich @ 2018-05-02 14:25 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization,
	virtio-dev, jasowang, nhorman, marcelo.leitner
In-Reply-To: <20180502170424-mutt-send-email-mst@kernel.org>

On 05/02/2018 10:17 AM, Michael S. Tsirkin wrote:
> On Wed, May 02, 2018 at 10:00:14AM -0400, Vlad Yasevich wrote:
>> On 05/02/2018 09:46 AM, Michael S. Tsirkin wrote:
>>> On Wed, May 02, 2018 at 09:27:00AM -0400, Vlad Yasevich wrote:
>>>> On 05/01/2018 11:24 PM, Michael S. Tsirkin wrote:
>>>>> On Tue, May 01, 2018 at 10:07:38PM -0400, Vladislav Yasevich wrote:
>>>>>> Since we now have support for software CRC32c offload, turn it on
>>>>>> for macvlan and macvtap devices so that guests can take advantage
>>>>>> of offload SCTP checksums to the host or host hardware.
>>>>>>
>>>>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>>>>> ---
>>>>>>  drivers/net/macvlan.c | 5 +++--
>>>>>>  drivers/net/tap.c     | 8 +++++---
>>>>>>  2 files changed, 8 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>>>>> index 725f4b4..646b730 100644
>>>>>> --- a/drivers/net/macvlan.c
>>>>>> +++ b/drivers/net/macvlan.c
>>>>>> @@ -834,7 +834,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
>>>>>>  
>>>>>>  #define ALWAYS_ON_OFFLOADS \
>>>>>>  	(NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \
>>>>>> -	 NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL)
>>>>>> +	 NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL | NETIF_F_SCTP_CRC)
>>>>>>  
>>>>>>  #define ALWAYS_ON_FEATURES (ALWAYS_ON_OFFLOADS | NETIF_F_LLTX)
>>>>>>  
>>>>>> @@ -842,7 +842,8 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
>>>>>>  	(NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
>>>>>>  	 NETIF_F_GSO | NETIF_F_TSO | NETIF_F_LRO | \
>>>>>>  	 NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \
>>>>>> -	 NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER)
>>>>>> +	 NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER | \
>>>>>> +	 NETIF_F_SCTP_CRC)
>>>>>>  
>>>>>>  #define MACVLAN_STATE_MASK \
>>>>>>  	((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))
>>>>>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>>>>>> index 9b6cb78..2c8512b 100644
>>>>>> --- a/drivers/net/tap.c
>>>>>> +++ b/drivers/net/tap.c
>>>>>> @@ -369,8 +369,7 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb)
>>>>>>  		 *	  check, we either support them all or none.
>>>>>>  		 */
>>>>>>  		if (skb->ip_summed == CHECKSUM_PARTIAL &&
>>>>>> -		    !(features & NETIF_F_CSUM_MASK) &&
>>>>>> -		    skb_checksum_help(skb))
>>>>>> +		    skb_csum_hwoffload_help(skb, features))
>>>>>>  			goto drop;
>>>>>>  		if (ptr_ring_produce(&q->ring, skb))
>>>>>>  			goto drop;
>>>>>> @@ -945,6 +944,9 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>>>>>>  		}
>>>>>>  	}
>>>>>>  
>>>>>> +	if (arg & TUN_F_SCTP_CSUM)
>>>>>> +		feature_mask |= NETIF_F_SCTP_CRC;
>>>>>> +
>>>>>
>>>>> so this still affects TX, shouldn't this affect RX instead?
>>>>
>>>> There is no bit to set on the RX path just like there is no bit to set on the RX patch
>>>> for TUN_F_CSUM.
>>>>
>>>> We only invert TSO offloads, not checksum offloads as the comment below states.
>>>> For checksum,  macvtap has to compute the checksum itself in tap_handle_frame() above.
>>>> It uses tx feature bits to see if needs do to the checksum.
>>>>
>>>> If you think we need another flag to macvtap to control RXCSUM, that would need to be
>>>> separate and cover standard TCP checksum as well.
>>>>
>>>> -vlad
>>>
>>> Confused. What is the meaning of TUN_F_SCTP_CSUM? I assume this is
>>> a way for userspace to tell tun device: "I can handle
>>> packets without SCTP checksum, pls send them my way".
>>
>> Yes,  just as TUN_F_CSUM means that tun device can handle packets with
>> partial tcp/udp checksum.
>>
>>>
>>> Now what is the implication for macvtap? 
>>
>> The implication is exactly the same as for TUN_F_CSUM.  If the
>> flag is set on the macvtap device, the TX checksum feature is
>> turned on.
> 
> I guess I will have to go back and re-read that code - I do not remember
> what does TUN_F_CSUM does by now.  Here is a quick question that might
> help me:
> 
> Let's assume userspace does not set TUN_F_SCTP_CSUM and device
> does not calculate the checksums either. I would expect that with
> macvtap this then behaves exactly like now with no overhead.

correct.

> 
>>
>>> And why  are
>>> you setting NETIF_F_SCTP_CRC which is a flag
>>> that affects packets sent by guest to host?
>>
>> Mainly its because we are using just 1 flag to control checksum
>> offloading and we need to be able control both tx and rx paths.
> 
> Well that's not really the case I think. What we have is controls for tx
> offloads for tun. That's TUN_F_CSUM.
> there are no rx offloads - userspace can send what it wants.
> 
> These are supposed to translate to rx offloads for macvtap.
> tx offloads shouldn't be affected at all.

Ok.  This is how it works for TSO, but looks like there was a disconnect
on the checksum.   In both tun and macvtap case, TUN_F_CSUM controls
the tx checksum feature bit.

> 
> Maybe that's not the case - as I said need to go back and check.
> Will try to find the time in the next couple of days.
> 
>> What you are suggesting that we either invert what TUN_F_CSUM
>> is doing in macvtap case, or have another flag that lets us control
>> TX and RX paths separately.
>>
>> Either case, that would be separate work.
>> -vlad
> 
> So assuming TUN_F_CSUM affects tx for macvtap do you
> agree it's a bug? And should we add to it with
> TUN_F_SCTP_CSUM doing the same?

let me think about what it would look like if TUN_F_CSUM
controlled the rx bits...

-vlad

> 
>>>
>>>
>>>>>
>>>>>
>>>>>>  	/* tun/tap driver inverts the usage for TSO offloads, where
>>>>>>  	 * setting the TSO bit means that the userspace wants to
>>>>>>  	 * accept TSO frames and turning it off means that user space
>>>>>> @@ -1077,7 +1079,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
>>>>>>  	case TUNSETOFFLOAD:
>>>>>>  		/* let the user check for future flags */
>>>>>>  		if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
>>>>>> -			    TUN_F_TSO_ECN | TUN_F_UFO))
>>>>>> +			    TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_SCTP_CSUM))
>>>>>>  			return -EINVAL;
>>>>>>  
>>>>>>  		rtnl_lock();
>>>>>> -- 
>>>>>> 2.9.5

^ permalink raw reply

* Re: [PATCH v2] net/mlx4_en: fix potential use-after-free with dma_unmap_page
From: David Miller @ 2018-05-02 14:26 UTC (permalink / raw)
  To: tariqt; +Cc: srn, yishaih, netdev
In-Reply-To: <5ee8574e-154c-3fa6-8b29-09fae1d08861@mellanox.com>

From: Tariq Toukan <tariqt@mellanox.com>
Date: Wed, 2 May 2018 16:50:28 +0300

> This patch fixes an issue existing in old kernels. It is not relevant
> per latest code.
> 
> So I'm not sure about the process. After I review it, do I just submit
> it again for -stable?

If that's the case, yes that is what you do, just submit it for -stable
and add a note to the top of the commit message body that says something
like:

	[ Not relevant upstream, therefore no upstream commit. ]

As -stable submissions have to say what their upstream commit ID is
otherwise.

CC: me on the submission, thank you.

Thanks.

^ permalink raw reply

* Re: [PATCH] Revert "vhost: make msg padding explicit"
From: David Miller @ 2018-05-02 14:27 UTC (permalink / raw)
  To: mst; +Cc: linux-kernel, jasowang, kvm, virtualization, netdev
In-Reply-To: <1525270740-460646-1-git-send-email-mst@redhat.com>

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Wed, 2 May 2018 17:19:05 +0300

> This reverts commit 93c0d549c4c5a7382ad70de6b86610b7aae57406.
> 
> Unfortunately the padding will break 32 bit userspace.
> Ouch. Need to add some compat code, revert for now.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Applied, thanks Michael.

^ permalink raw reply

* Re: [PATCH RFC iproute2-next 2/2] rdma: print provider resource attributes
From: Stephen Hemminger @ 2018-05-02 14:32 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Steve Wise, dsahern, netdev, linux-rdma
In-Reply-To: <20180502133852.GK20375@mtr-leonro.local>

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

On Wed, 2 May 2018 16:38:52 +0300
Leon Romanovsky <leon@kernel.org> wrote:

> On Mon, Apr 30, 2018 at 08:25:24AM -0700, Stephen Hemminger wrote:
> > On Mon, 30 Apr 2018 07:36:18 -0700
> > Steve Wise <swise@opengridcomputing.com> wrote:
> >  
> > > +#define nla_type(attr) ((attr)->nla_type & NLA_TYPE_MASK)
> > > +
> > > +void newline(struct rd *rd)
> > > +{
> > > +	if (rd->json_output)
> > > +		jsonw_end_array(rd->jw);
> > > +	else
> > > +		pr_out("\n");
> > > +}
> > > +
> > > +void newline_indent(struct rd *rd)
> > > +{
> > > +	newline(rd);
> > > +	if (!rd->json_output)
> > > +		pr_out("    ");
> > > +}
> > > +
> > > +static int print_provider_string(struct rd *rd, const char *key_str,
> > > +				 const char *val_str)
> > > +{
> > > +	if (rd->json_output) {
> > > +		jsonw_string_field(rd->jw, key_str, val_str);
> > > +		return 0;
> > > +	} else {
> > > +		return pr_out("%s %s ", key_str, val_str);
> > > +	}
> > > +}
> > > +
> > > +static int print_provider_s32(struct rd *rd, const char *key_str, int32_t val,
> > > +			      enum rdma_nldev_print_type print_type)
> > > +{
> > > +	if (rd->json_output) {
> > > +		jsonw_int_field(rd->jw, key_str, val);
> > > +		return 0;
> > > +	}
> > > +	switch (print_type) {
> > > +	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
> > > +		return pr_out("%s %d ", key_str, val);
> > > +	case RDMA_NLDEV_PRINT_TYPE_HEX:
> > > +		return pr_out("%s 0x%x ", key_str, val);
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +  
> >
> > This code should get converted to json_print library that handles the
> > different output modes; rather than rolling it's own equivalent functionality.  
> 
> Can it be done after this patch is merged? It will simplify review and
> testing because current code is implemented to be in the same format as
> the rest of the tool.
> 
> Thanks

Sure, I was just encouraging future direction.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH V2 net-next 1/6] virtio: Add support for SCTP checksum offloading
From: Marcelo Ricardo Leitner @ 2018-05-02 14:34 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Vladislav Yasevich, netdev, linux-sctp, virtualization,
	virtio-dev, jasowang, nhorman, Vladislav Yasevich
In-Reply-To: <20180502171943-mutt-send-email-mst@kernel.org>

On Wed, May 02, 2018 at 05:21:38PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 02, 2018 at 11:14:13AM -0300, Marcelo Ricardo Leitner wrote:
> > On Wed, May 02, 2018 at 06:16:45AM +0300, Michael S. Tsirkin wrote:
> > > On Tue, May 01, 2018 at 10:07:34PM -0400, Vladislav Yasevich wrote:
> > > > To support SCTP checksum offloading, we need to add a new feature
> > > > to virtio_net, so we can negotiate support between the hypervisor
> > > > and the guest.
> > > > The HOST feature bit signifies offloading support for transmit and
> > > > enables device offload features.
> > > > The GUEST feature bit signifies offloading support of recieve and
> > > > is currently only used by the driver in case of xdp.
> > > >
> > > > That patch also adds an addition virtio_net header flag which
> > > > mirrors the skb->csum_not_inet flag.  This flags is used to indicate
> > > > that is this an SCTP packet that needs its checksum computed by the
> > > > lower layer.  In this case, the lower layer is the host hypervisor or
> > > > possibly HW nic that supporst CRC32c offload.
> > > >
> > > > In the case that GUEST feature bit is flag, it will be possible to
> > > > receive a virtio_net header with this bit set, which will set the
> > > > corresponding skb bit.  SCTP protocol will be updated to correctly
> > > > deal with it.
> > > >
> > > > Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> > > > ---
> > > >  drivers/net/virtio_net.c        | 14 +++++++++++++-
> > > >  include/linux/virtio_net.h      |  6 ++++++
> > > >  include/uapi/linux/virtio_net.h |  5 +++++
> > > >  3 files changed, 24 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 7b187ec..34af280 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -2148,6 +2148,8 @@ static int virtnet_clear_guest_offloads(struct virtnet_info *vi)
> > > >
> > > >  	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM))
> > > >  		offloads = 1ULL << VIRTIO_NET_F_GUEST_CSUM;
> > > > +	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_SCTP_CSUM))
> > > > +		offloads |= 1ULL << VIRTIO_NET_F_GUEST_SCTP_CSUM;
> > > >
> > > >  	return virtnet_set_guest_offloads(vi, offloads);
> > > >  }
> > > > @@ -2160,6 +2162,8 @@ static int virtnet_restore_guest_offloads(struct virtnet_info *vi)
> > > >  		return 0;
> > > >  	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM))
> > > >  		offloads |= 1ULL << VIRTIO_NET_F_GUEST_CSUM;
> > > > +	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_SCTP_CSUM))
> > > > +		offloads |= 1ULL << VIRTIO_NET_F_GUEST_SCTP_CSUM;
> > > >
> > > >  	return virtnet_set_guest_offloads(vi, offloads);
> > > >  }
> > > > @@ -2724,6 +2728,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> > > >  	/* Do we support "hardware" checksums? */
> > > >  	if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> > > >  		/* This opens up the world of extra features. */
> > > > +
> > > >  		dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG;
> > > >  		if (csum)
> > > >  			dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG;
> > > > @@ -2746,9 +2751,15 @@ static int virtnet_probe(struct virtio_device *vdev)
> > > >  			dev->features |= dev->hw_features & NETIF_F_ALL_TSO;
> > > >  		/* (!csum && gso) case will be fixed by register_netdev() */
> > > >  	}
> > > > +
> > > >  	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
> > > >  		dev->features |= NETIF_F_RXCSUM;
> > > >
> > > > +	if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_SCTP_CSUM)) {
> > > > +		dev->hw_features |= NETIF_F_SCTP_CRC;
> > > > +		dev->features |= NETIF_F_SCTP_CRC;
> > > > +	}
> > > > +
> > > >  	dev->vlan_features = dev->features;
> > > >
> > > >  	/* MTU range: 68 - 65535 */
> > > > @@ -2962,7 +2973,8 @@ static struct virtio_device_id id_table[] = {
> > > >  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> > > >  	VIRTIO_NET_F_CTRL_MAC_ADDR, \
> > > >  	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> > > > -	VIRTIO_NET_F_SPEED_DUPLEX
> > > > +	VIRTIO_NET_F_SPEED_DUPLEX, \
> > > > +	VIRTIO_NET_F_HOST_SCTP_CSUM, VIRTIO_NET_F_GUEST_SCTP_CSUM
> > > >
> > > >  static unsigned int features[] = {
> > > >  	VIRTNET_FEATURES,
> > > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > > > index f144216..28fffdc 100644
> > > > --- a/include/linux/virtio_net.h
> > > > +++ b/include/linux/virtio_net.h
> > > > @@ -39,6 +39,9 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
> > > >
> > > >  		if (!skb_partial_csum_set(skb, start, off))
> > > >  			return -EINVAL;
> > > > +
> > > > +		if (hdr->flags & VIRTIO_NET_HDR_F_CSUM_NOT_INET)
> > > > +			skb->csum_not_inet = 1;
> > > >  	}
> > > >
> > > >  	if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> > > > @@ -91,6 +94,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
> > > >  				skb_checksum_start_offset(skb));
> > > >  		hdr->csum_offset = __cpu_to_virtio16(little_endian,
> > > >  				skb->csum_offset);
> > > > +
> > > > +		if (skb->csum_not_inet)
> > > > +			hdr->flags |= VIRTIO_NET_HDR_F_CSUM_NOT_INET;
> > > >  	} else if (has_data_valid &&
> > > >  		   skb->ip_summed == CHECKSUM_UNNECESSARY) {
> > > >  		hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
> > > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > > > index 5de6ed3..9dfca1a 100644
> > > > --- a/include/uapi/linux/virtio_net.h
> > > > +++ b/include/uapi/linux/virtio_net.h
> > > > @@ -57,6 +57,10 @@
> > > >  					 * Steering */
> > > >  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
> > > >
> > > > +#define VIRTIO_NET_F_GUEST_SCTP_CSUM  61 /* Guest handles SCTP pks w/ partial
> > > > +					  * csum */
> > > > +#define VIRTIO_NET_F_HOST_SCTP_CSUM   62 /* HOST handles SCTP pkts w/ partial
> > > > +					  * csum */
> > > >  #define VIRTIO_NET_F_SPEED_DUPLEX 63	/* Device set linkspeed and duplex */
> > > >
> > > >  #ifndef VIRTIO_NET_NO_LEGACY
> > > > @@ -101,6 +105,7 @@ struct virtio_net_config {
> > > >  struct virtio_net_hdr_v1 {
> > > >  #define VIRTIO_NET_HDR_F_NEEDS_CSUM	1	/* Use csum_start, csum_offset */
> > > >  #define VIRTIO_NET_HDR_F_DATA_VALID	2	/* Csum is valid */
> > > > +#define VIRTIO_NET_HDR_F_CSUM_NOT_INET  4       /* Checksum is not inet */
> > >
> > > Both comment and name are not very informative.
> > > How about just saying CRC32c ?
> >
> > csum_not_inet is following the nomenclature used in sk_buff. Initially
> > Davide had named the sk_buff field after crc32c but then it was argued
> > that maybe another check method may be introduced later and the name
> > would be bogus, thus why the "csum_not_inet".
>
> That's sk_buff internals, since Linux can change these
> at any time without breaking anything.
>
> virtio defines an ABI so we won't be able to make it
> mean something else, need to document it fully.
> "Go read linux net core code" is not a good answer
> to the natural question "what does this bit in the
> interface do".

Fair enough, but as I just said that is exactly why it was named
'csum_not_inet' in the first place, so that it wouldn't have to change
if another method needs to be added.

But I guess your main point is that virtio can have a cleaner ABI and
skip this csum_not_inet case (which is only there because sk_buff
doesn't have as many bits as we would want), right?

>
> > >
> > > >  	__u8 flags;
> > > >  #define VIRTIO_NET_HDR_GSO_NONE		0	/* Not a GSO frame */
> > > >  #define VIRTIO_NET_HDR_GSO_TCPV4	1	/* GSO frame, IPv4 TCP (TSO) */
> > > > --
> > > > 2.9.5
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH V2 net-next 2/6] sctp: Handle sctp packets with CHECKSUM_PARTIAL
From: Marcelo Ricardo Leitner @ 2018-05-02 14:38 UTC (permalink / raw)
  To: Vladislav Yasevich
  Cc: netdev, linux-sctp, virtualization, virtio-dev, mst, jasowang,
	nhorman, Vladislav Yasevich
In-Reply-To: <20180502020739.19239-3-vyasevic@redhat.com>

On Tue, May 01, 2018 at 10:07:35PM -0400, Vladislav Yasevich wrote:
> With SCTP checksum offload available in virtio, it is now
> possible for virtio to receive a sctp packet with CHECKSUM_PARTIAL
> set (guest-to-guest traffic).  SCTP doesn't really have a
> partial checksum like TCP does, because CRC32c can't do partial
> additive checksumming.  It's all or nothing.  So an SCTP packet
> with CHECKSUM_PARTIAL will have checksum set to 0.  Let SCTP
> treat this as a valid checksum if CHECKSUM_PARTIAL is set.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
>  net/sctp/input.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index ba8a6e6..055b8ffa 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -80,8 +80,17 @@ static inline int sctp_rcv_checksum(struct net *net, struct sk_buff *skb)
>  {
>  	struct sctphdr *sh = sctp_hdr(skb);
>  	__le32 cmp = sh->checksum;
> -	__le32 val = sctp_compute_cksum(skb, 0);
> +	__le32 val = 0;
>
> +	/* In sctp PARTIAL checksum is always 0.  This is a case of
> +	 * a packet received from guest that supports checksum offload.
> +	 * Assume it's correct as there is really no way to verify,
> +	 * and we want to avaoid computing it unnecesarily.
               nit, typos --^                     --^
> +	 */
> +	if (skb->ip_summed == CHECKSUM_PARTIAL)
> +		return 0;
> +
> +	val = sctp_compute_cksum(skb, 0);
>  	if (val != cmp) {
>  		/* CRC failure, dump it. */
>  		__SCTP_INC_STATS(net, SCTP_MIB_CHECKSUMERRORS);
> --
> 2.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH] vhost: make msg padding explicit
From: Kevin Easton @ 2018-05-02 14:40 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David Miller, linux-kernel, jasowang, kvm, virtualization, netdev
In-Reply-To: <20180502171909-mutt-send-email-mst@kernel.org>

On Wed, May 02, 2018 at 05:19:27PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 02, 2018 at 10:04:46AM -0400, David Miller wrote:
> > From: "Michael S. Tsirkin" <mst@redhat.com>
> > Date: Wed, 2 May 2018 16:36:37 +0300
> > 
> > > Ouch.  True - and in particular the 32 bit ABI on 64 bit kernels doesn't
> > > work at all. Hmm. It's relatively new and maybe there aren't any 32 bit
> > > users yet. Thoughts?
> > 
> > If it's been in a released kernel version, we really aren't at liberty
> > to play "maybe nobody uses this" UAPI changing games.
> > 
> > Please send me a revert.
> 
> Sent. Looking at compat mess now.

Just naming the padding field isn't sufficient anyway, there also needs to
be code in vhost_new_msg() to initialise it using the name.

    - Kevin

^ permalink raw reply

* Re: [PATCH V2 6/8] net: stmmac: add dwmac-4.20a compatible
From: Jose Abreu @ 2018-05-02 14:41 UTC (permalink / raw)
  To: Christophe Roullier, mark.rutland, mcoquelin.stm32,
	alexandre.torgue, peppe.cavallaro
  Cc: devicetree, linux-arm-kernel, netdev, andrew
In-Reply-To: <1525270723-18241-7-git-send-email-christophe.roullier@st.com>

Hi Christophe,

On 02-05-2018 15:18, Christophe Roullier wrote:
> Manage dwmac-4.20a version from synopsys
>
>

Just being curious: Can you tell me which HW features do you have
on your NIC?

Thanks and Best Regards,
Jose Miguel Abreu

^ 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