* [PATCH net-next] mvpp2: document HW checksum behaviour
@ 2019-07-25 23:15 Matteo Croce
2019-07-26 12:57 ` Antoine Tenart
0 siblings, 1 reply; 7+ messages in thread
From: Matteo Croce @ 2019-07-25 23:15 UTC (permalink / raw)
To: netdev
Cc: Antoine Tenart, Maxime Chevallier, Marcin Wojtas, Stefan Chulski, LKML
The hardware can only offload checksum calculation on first port due to
the Tx FIFO size limitation. Document this in a comment.
Fixes: 576193f2d579 ("net: mvpp2: jumbo frames support")
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index d8e5241097a9..2f7286bd203b 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -843,7 +843,10 @@ static int mvpp2_bm_update_mtu(struct net_device *dev, int mtu)
/* Add port to new short & long pool */
mvpp2_swf_bm_pool_init(port);
- /* Update L4 checksum when jumbo enable/disable on port */
+ /* Update L4 checksum when jumbo enable/disable on port.
+ * Only port 0 supports hardware checksum offload due to
+ * the Tx FIFO size limitation.
+ */
if (new_long_pool == MVPP2_BM_JUMBO && port->id != 0) {
dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
dev->hw_features &= ~(NETIF_F_IP_CSUM |
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net-next] mvpp2: document HW checksum behaviour 2019-07-25 23:15 [PATCH net-next] mvpp2: document HW checksum behaviour Matteo Croce @ 2019-07-26 12:57 ` Antoine Tenart 2019-07-26 14:35 ` Matteo Croce 2019-07-28 1:36 ` Matteo Croce 0 siblings, 2 replies; 7+ messages in thread From: Antoine Tenart @ 2019-07-26 12:57 UTC (permalink / raw) To: Matteo Croce Cc: netdev, Antoine Tenart, Maxime Chevallier, Marcin Wojtas, Stefan Chulski, LKML Hi Matteo, On Fri, Jul 26, 2019 at 01:15:46AM +0200, Matteo Croce wrote: > The hardware can only offload checksum calculation on first port due to > the Tx FIFO size limitation. Document this in a comment. > > Fixes: 576193f2d579 ("net: mvpp2: jumbo frames support") > Signed-off-by: Matteo Croce <mcroce@redhat.com> Looks good. Please note there's a similar code path in the probe. You could also add a comment there (or move this check/comment in a common place). Thanks! Antoine > --- > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > index d8e5241097a9..2f7286bd203b 100644 > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > @@ -843,7 +843,10 @@ static int mvpp2_bm_update_mtu(struct net_device *dev, int mtu) > /* Add port to new short & long pool */ > mvpp2_swf_bm_pool_init(port); > > - /* Update L4 checksum when jumbo enable/disable on port */ > + /* Update L4 checksum when jumbo enable/disable on port. > + * Only port 0 supports hardware checksum offload due to > + * the Tx FIFO size limitation. > + */ > if (new_long_pool == MVPP2_BM_JUMBO && port->id != 0) { > dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); > dev->hw_features &= ~(NETIF_F_IP_CSUM | > -- > 2.21.0 > -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] mvpp2: document HW checksum behaviour 2019-07-26 12:57 ` Antoine Tenart @ 2019-07-26 14:35 ` Matteo Croce 2019-07-27 20:23 ` David Miller 2019-07-28 1:36 ` Matteo Croce 1 sibling, 1 reply; 7+ messages in thread From: Matteo Croce @ 2019-07-26 14:35 UTC (permalink / raw) To: Antoine Tenart Cc: netdev, Maxime Chevallier, Marcin Wojtas, Stefan Chulski, LKML On Fri, Jul 26, 2019 at 2:57 PM Antoine Tenart <antoine.tenart@bootlin.com> wrote: > > Hi Matteo, > > On Fri, Jul 26, 2019 at 01:15:46AM +0200, Matteo Croce wrote: > > The hardware can only offload checksum calculation on first port due to > > the Tx FIFO size limitation. Document this in a comment. > > > > Fixes: 576193f2d579 ("net: mvpp2: jumbo frames support") > > Signed-off-by: Matteo Croce <mcroce@redhat.com> > > Looks good. Please note there's a similar code path in the probe. You > could also add a comment there (or move this check/comment in a common > place). > > Thanks! > Antoine > > > --- > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > index d8e5241097a9..2f7286bd203b 100644 > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c > > @@ -843,7 +843,10 @@ static int mvpp2_bm_update_mtu(struct net_device *dev, int mtu) > > /* Add port to new short & long pool */ > > mvpp2_swf_bm_pool_init(port); > > > > - /* Update L4 checksum when jumbo enable/disable on port */ > > + /* Update L4 checksum when jumbo enable/disable on port. > > + * Only port 0 supports hardware checksum offload due to > > + * the Tx FIFO size limitation. > > + */ > > if (new_long_pool == MVPP2_BM_JUMBO && port->id != 0) { > > dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); > > dev->hw_features &= ~(NETIF_F_IP_CSUM | > > -- > > 2.21.0 > > > > -- > Antoine Ténart, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com I see, there is a similar statement in mvpp2_port_probe(). What about adding a static function which sets the flag, and add the comment there instead of duplicating the comment? -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] mvpp2: document HW checksum behaviour 2019-07-26 14:35 ` Matteo Croce @ 2019-07-27 20:23 ` David Miller 0 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2019-07-27 20:23 UTC (permalink / raw) To: mcroce Cc: antoine.tenart, netdev, maxime.chevallier, mw, stefanc, linux-kernel From: Matteo Croce <mcroce@redhat.com> Date: Fri, 26 Jul 2019 16:35:59 +0200 > I see, there is a similar statement in mvpp2_port_probe(). > What about adding a static function which sets the flag, and add the > comment there instead of duplicating the comment? That sounds good to me. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] mvpp2: document HW checksum behaviour 2019-07-26 12:57 ` Antoine Tenart 2019-07-26 14:35 ` Matteo Croce @ 2019-07-28 1:36 ` Matteo Croce 2019-07-28 14:30 ` Matteo Croce 1 sibling, 1 reply; 7+ messages in thread From: Matteo Croce @ 2019-07-28 1:36 UTC (permalink / raw) To: Antoine Tenart, Marcin Wojtas, Stefan Chulski, Maxime Chevallier Cc: netdev, LKML, David S . Miller On Fri, Jul 26, 2019 at 2:57 PM Antoine Tenart <antoine.tenart@bootlin.com> wrote: > > Hi Matteo, > > On Fri, Jul 26, 2019 at 01:15:46AM +0200, Matteo Croce wrote: > > The hardware can only offload checksum calculation on first port due to > > the Tx FIFO size limitation. Document this in a comment. > > > > Fixes: 576193f2d579 ("net: mvpp2: jumbo frames support") > > Signed-off-by: Matteo Croce <mcroce@redhat.com> > > Looks good. Please note there's a similar code path in the probe. You > could also add a comment there (or move this check/comment in a common > place). > > Thanks! > Antoine > Hi Antoine, I was making a v2, when I looked at the mvpp2_port_probe() which does: --------------------------------%<------------------------------ features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_TSO; if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) { dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); dev->hw_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); } dev->vlan_features |= features; -------------------------------->%------------------------------ Is it ok to remove NETIF_F_IP*_CSUM from dev->features and dev->hw_features but keep it in dev->vlan_features? Regards, -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] mvpp2: document HW checksum behaviour 2019-07-28 1:36 ` Matteo Croce @ 2019-07-28 14:30 ` Matteo Croce 2019-07-28 15:22 ` [EXT] " Stefan Chulski 0 siblings, 1 reply; 7+ messages in thread From: Matteo Croce @ 2019-07-28 14:30 UTC (permalink / raw) To: Antoine Tenart, Marcin Wojtas, Stefan Chulski, Maxime Chevallier Cc: netdev, LKML, David S . Miller On Sun, Jul 28, 2019 at 3:36 AM Matteo Croce <mcroce@redhat.com> wrote: > > On Fri, Jul 26, 2019 at 2:57 PM Antoine Tenart > <antoine.tenart@bootlin.com> wrote: > > > > Hi Matteo, > > > > On Fri, Jul 26, 2019 at 01:15:46AM +0200, Matteo Croce wrote: > > > The hardware can only offload checksum calculation on first port > > > due to the Tx FIFO size limitation. Document this in a comment. > > > > > > Fixes: 576193f2d579 ("net: mvpp2: jumbo frames support") > > > Signed-off-by: Matteo Croce <mcroce@redhat.com> > > > > Looks good. Please note there's a similar code path in the probe. > > You could also add a comment there (or move this check/comment in a > > common place). > > > > Thanks! > > Antoine > > > > Hi Antoine, > > I was making a v2, when I looked at the mvpp2_port_probe() which does: > > --------------------------------%<------------------------------ > features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | > NETIF_F_TSO; > > if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) { > dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); > dev->hw_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); > } > > dev->vlan_features |= features; > -------------------------------->%------------------------------ > > Is it ok to remove NETIF_F_IP*_CSUM from dev->features and > dev->hw_features but keep it in dev->vlan_features? > > Regards, > -- > Matteo Croce > per aspera ad upstream Hi all, probably dev->vlan_features is safe to keep the CSUM features to avoid unnecessary calculation in some cases, but I have another question. Does the PP2 hardware support checksumming within any offset? I replaced 'NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM' with NETIF_F_HW_CSUM and then stacked 5 VxLANS on top of a mvpp2 device, to have the last IP header at offset 264: ip link set $dev up ip addr add 192.168.0.$last/24 dev $dev for i in {1..5}; do ip link add vx$i type vxlan id $i dstport 4789 remote 192.168.$((i-1)).$other ip link set vx$i up ip addr add 192.168.$i.$last/24 dev vx$i done 00:51:82:11:22:00 > 3c:fd:fe:9c:60:6c, ethertype IPv4 (0x0800), length 348: 192.168.0.1.33625 > 192.168.0.2.4789: VXLAN, flags [I] (0x08), vni 1 02:25:60:da:87:03 > 92:20:05:45:3d:d3, ethertype IPv4 (0x0800), length 298: 192.168.1.1.33625 > 192.168.1.2.4789: VXLAN, flags [I] (0x08), vni 2 12:20:97:15:8f:aa > 66:08:23:c7:72:ea, ethertype IPv4 (0x0800), length 248: 192.168.2.1.33625 > 192.168.2.2.4789: VXLAN, flags [I] (0x08), vni 3 c6:1c:b9:fd:9d:28 > 22:ca:cb:6a:ea:68, ethertype IPv4 (0x0800), length 198: 192.168.3.1.33625 > 192.168.3.2.4789: VXLAN, flags [I] (0x08), vni 4 02:34:5f:45:a5:9d > d2:4e:d4:d7:42:31, ethertype IPv4 (0x0800), length 148: 192.168.4.1.34504 > 192.168.4.2.4789: VXLAN, flags [I] (0x08), vni 5 a2:99:fd:9c:1b:05 > 5a:81:3b:fc:6a:07, ethertype IPv4 (0x0800), length 98: 192.168.5.1 > 192.168.5.2: ICMP echo request, id 1654, seq 156, length 64 It seems that the HW is capable of doing it, can someone with a datasheet confirm this? Regards, -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [EXT] Re: [PATCH net-next] mvpp2: document HW checksum behaviour 2019-07-28 14:30 ` Matteo Croce @ 2019-07-28 15:22 ` Stefan Chulski 0 siblings, 0 replies; 7+ messages in thread From: Stefan Chulski @ 2019-07-28 15:22 UTC (permalink / raw) To: Matteo Croce, Antoine Tenart, Marcin Wojtas, Maxime Chevallier Cc: netdev, LKML, David S . Miller > Hi all, > > probably dev->vlan_features is safe to keep the CSUM features to avoid > unnecessary calculation in some cases, but I have another question. > Does the PP2 hardware support checksumming within any offset? I replaced > 'NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM' with NETIF_F_HW_CSUM and > then stacked 5 VxLANS on top of a mvpp2 device, to have the last IP header > at offset 264: > > ip link set $dev up > ip addr add 192.168.0.$last/24 dev $dev > > for i in {1..5}; do > ip link add vx$i type vxlan id $i dstport 4789 remote 192.168.$((i- > 1)).$other > ip link set vx$i up > ip addr add 192.168.$i.$last/24 dev vx$i done > > 00:51:82:11:22:00 > 3c:fd:fe:9c:60:6c, ethertype IPv4 (0x0800), length 348: > 192.168.0.1.33625 > 192.168.0.2.4789: VXLAN, flags [I] (0x08), vni 1 > 02:25:60:da:87:03 > 92:20:05:45:3d:d3, ethertype IPv4 (0x0800), length 298: > 192.168.1.1.33625 > 192.168.1.2.4789: VXLAN, flags [I] (0x08), vni 2 > 12:20:97:15:8f:aa > 66:08:23:c7:72:ea, ethertype IPv4 (0x0800), length 248: > 192.168.2.1.33625 > 192.168.2.2.4789: VXLAN, flags [I] (0x08), vni 3 > c6:1c:b9:fd:9d:28 > 22:ca:cb:6a:ea:68, ethertype IPv4 (0x0800), length 198: > 192.168.3.1.33625 > 192.168.3.2.4789: VXLAN, flags [I] (0x08), vni 4 > 02:34:5f:45:a5:9d > d2:4e:d4:d7:42:31, ethertype IPv4 (0x0800), length 148: > 192.168.4.1.34504 > 192.168.4.2.4789: VXLAN, flags [I] (0x08), vni 5 > a2:99:fd:9c:1b:05 > 5a:81:3b:fc:6a:07, ethertype IPv4 (0x0800), length 98: > 192.168.5.1 > 192.168.5.2: ICMP echo request, id 1654, seq 156, length 64 > > It seems that the HW is capable of doing it, can someone with a datasheet > confirm this? L3_offset in TX descriptor has 7 bits, so beginning of Layer3 should be less than 128 Bytes. Stefan, Regards. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-07-28 15:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-25 23:15 [PATCH net-next] mvpp2: document HW checksum behaviour Matteo Croce 2019-07-26 12:57 ` Antoine Tenart 2019-07-26 14:35 ` Matteo Croce 2019-07-27 20:23 ` David Miller 2019-07-28 1:36 ` Matteo Croce 2019-07-28 14:30 ` Matteo Croce 2019-07-28 15:22 ` [EXT] " Stefan Chulski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).