* [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments @ 2018-03-27 21:21 Eric Dumazet 2018-03-29 18:07 ` David Miller 2018-03-30 0:34 ` Saeed Mahameed 0 siblings, 2 replies; 8+ messages in thread From: Eric Dumazet @ 2018-03-27 21:21 UTC (permalink / raw) To: David S . Miller Cc: netdev, Eric Dumazet, Eric Dumazet, Willem de Bruijn, Tariq Toukan Refine the RX check summing handling to propagate the hardware provided checksum so that we do not have to compute it later in software. Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Willem de Bruijn <willemb@google.com> Cc: Tariq Toukan <tariqt@mellanox.com> --- drivers/net/ethernet/mellanox/mlx4/en_rx.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c index 05787efef492b1c0c6ce540ef73647fad91ce282..5c613c6663da51a4ae792eeb4d8956b54655786b 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c @@ -821,14 +821,12 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud skb_record_rx_queue(skb, cq_ring); if (likely(dev->features & NETIF_F_RXCSUM)) { - if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP | - MLX4_CQE_STATUS_UDP)) { + if ((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP | + MLX4_CQE_STATUS_UDP)) && + (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) && + cqe->checksum == cpu_to_be16(0xffff)) { bool l2_tunnel; - if (!((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) && - cqe->checksum == cpu_to_be16(0xffff))) - goto csum_none; - l2_tunnel = (dev->hw_enc_features & NETIF_F_RXCSUM) && (cqe->vlan_my_qpn & cpu_to_be32(MLX4_CQE_L2_TUNNEL)); ip_summed = CHECKSUM_UNNECESSARY; -- 2.17.0.rc1.321.gba9d0f2565-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments 2018-03-27 21:21 [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments Eric Dumazet @ 2018-03-29 18:07 ` David Miller 2018-03-30 0:44 ` Saeed Mahameed 2018-03-30 0:34 ` Saeed Mahameed 1 sibling, 1 reply; 8+ messages in thread From: David Miller @ 2018-03-29 18:07 UTC (permalink / raw) To: edumazet; +Cc: netdev, eric.dumazet, willemb, tariqt From: Eric Dumazet <edumazet@google.com> Date: Tue, 27 Mar 2018 14:21:14 -0700 > Refine the RX check summing handling to propagate the > hardware provided checksum so that we do not have to > compute it later in software. > > Signed-off-by: Eric Dumazet <edumazet@google.com> Tariq, please review. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments 2018-03-29 18:07 ` David Miller @ 2018-03-30 0:44 ` Saeed Mahameed 2018-03-30 3:32 ` Eric Dumazet 0 siblings, 1 reply; 8+ messages in thread From: Saeed Mahameed @ 2018-03-30 0:44 UTC (permalink / raw) To: David Miller Cc: Eric Dumazet, Linux Netdev List, Eric Dumazet, Willem de Bruijn, Tariq Toukan On Thu, Mar 29, 2018 at 11:07 AM, David Miller <davem@davemloft.net> wrote: > From: Eric Dumazet <edumazet@google.com> > Date: Tue, 27 Mar 2018 14:21:14 -0700 > >> Refine the RX check summing handling to propagate the >> hardware provided checksum so that we do not have to >> compute it later in software. >> >> Signed-off-by: Eric Dumazet <edumazet@google.com> > > Tariq, please review. Hi Dave, Eric. The patch looks ok but i would let tariq review it and decide if he wants to run full regression coverage on it since it changes the default behavior of the driver's checksum reporting. It is already weekend for him and for the team in Israel, and i don't think this can be handled before next week :). So it is really up to you guys. Thanks, Saeed. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments 2018-03-30 0:44 ` Saeed Mahameed @ 2018-03-30 3:32 ` Eric Dumazet 0 siblings, 0 replies; 8+ messages in thread From: Eric Dumazet @ 2018-03-30 3:32 UTC (permalink / raw) To: Saeed Mahameed Cc: David Miller, netdev, Eric Dumazet, Willem de Bruijn, Tariq Toukan On Thu, Mar 29, 2018 at 5:44 PM Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote: > On Thu, Mar 29, 2018 at 11:07 AM, David Miller <davem@davemloft.net> wrote: > > From: Eric Dumazet <edumazet@google.com> > > Date: Tue, 27 Mar 2018 14:21:14 -0700 > > > >> Refine the RX check summing handling to propagate the > >> hardware provided checksum so that we do not have to > >> compute it later in software. > >> > >> Signed-off-by: Eric Dumazet <edumazet@google.com> > > > > Tariq, please review. > Hi Dave, Eric. > The patch looks ok but i would let tariq review it and decide if he > wants to run full regression coverage on it > since it changes the default behavior of the driver's checksum reporting. > It is already weekend for him and for the team in Israel, and i don't > think this can be handled before next week :). > So it is really up to you guys. > Thanks, > Saeed. Hi Saaed This definitely can wait, nothing urgent really. Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments 2018-03-27 21:21 [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments Eric Dumazet 2018-03-29 18:07 ` David Miller @ 2018-03-30 0:34 ` Saeed Mahameed 2018-04-01 8:06 ` Tariq Toukan 1 sibling, 1 reply; 8+ messages in thread From: Saeed Mahameed @ 2018-03-30 0:34 UTC (permalink / raw) To: Eric Dumazet Cc: David S . Miller, netdev, Eric Dumazet, Willem de Bruijn, Tariq Toukan On Tue, Mar 27, 2018 at 2:21 PM, Eric Dumazet <edumazet@google.com> wrote: > Refine the RX check summing handling to propagate the > hardware provided checksum so that we do not have to > compute it later in software. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Willem de Bruijn <willemb@google.com> > Cc: Tariq Toukan <tariqt@mellanox.com> > --- > drivers/net/ethernet/mellanox/mlx4/en_rx.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c > index 05787efef492b1c0c6ce540ef73647fad91ce282..5c613c6663da51a4ae792eeb4d8956b54655786b 100644 > --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c > +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c > @@ -821,14 +821,12 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud > skb_record_rx_queue(skb, cq_ring); > > if (likely(dev->features & NETIF_F_RXCSUM)) { > - if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP | > - MLX4_CQE_STATUS_UDP)) { > + if ((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP | > + MLX4_CQE_STATUS_UDP)) && > + (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) && > + cqe->checksum == cpu_to_be16(0xffff)) { > bool l2_tunnel; > LGTM, this code even aligns better with the mlx4 HW documentation: "When L4_CSUM field is not supported, L4 checksum for TCP/UDP packets can be validated by: (IP_OK && (TCP || UDP)) && (checksum == 0xFFFF))." in the code we don't even consider L4_CSUM at the moment, As a future patch, it could be a nice acceleration for the above 3 steps condition. Small comment, if we expect that cqe->checksum is NOT likely to be 0xffff for UDP/TCP packets, maybe it is better performance wise to move (cqe->checksum == cpu_to_be16(0xffff)) to be evaluated first in the condition. > - if (!((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) && > - cqe->checksum == cpu_to_be16(0xffff))) > - goto csum_none; > - > l2_tunnel = (dev->hw_enc_features & NETIF_F_RXCSUM) && > (cqe->vlan_my_qpn & cpu_to_be32(MLX4_CQE_L2_TUNNEL)); > ip_summed = CHECKSUM_UNNECESSARY; > -- > 2.17.0.rc1.321.gba9d0f2565-goog > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments 2018-03-30 0:34 ` Saeed Mahameed @ 2018-04-01 8:06 ` Tariq Toukan 2018-04-01 16:39 ` Eric Dumazet 0 siblings, 1 reply; 8+ messages in thread From: Tariq Toukan @ 2018-04-01 8:06 UTC (permalink / raw) To: Saeed Mahameed, Eric Dumazet Cc: David S . Miller, netdev, Eric Dumazet, Willem de Bruijn, Tariq Toukan On 30/03/2018 3:34 AM, Saeed Mahameed wrote: > On Tue, Mar 27, 2018 at 2:21 PM, Eric Dumazet <edumazet@google.com> wrote: >> Refine the RX check summing handling to propagate the >> hardware provided checksum so that we do not have to >> compute it later in software. >> >> Signed-off-by: Eric Dumazet <edumazet@google.com> >> Cc: Willem de Bruijn <willemb@google.com> >> Cc: Tariq Toukan <tariqt@mellanox.com> >> --- >> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 10 ++++------ >> 1 file changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c >> index 05787efef492b1c0c6ce540ef73647fad91ce282..5c613c6663da51a4ae792eeb4d8956b54655786b 100644 >> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c >> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c >> @@ -821,14 +821,12 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud >> skb_record_rx_queue(skb, cq_ring); >> >> if (likely(dev->features & NETIF_F_RXCSUM)) { >> - if (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP | >> - MLX4_CQE_STATUS_UDP)) { >> + if ((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_TCP | >> + MLX4_CQE_STATUS_UDP)) && >> + (cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) && >> + cqe->checksum == cpu_to_be16(0xffff)) { >> bool l2_tunnel; >> > > LGTM, this code even aligns better with the mlx4 HW documentation: > > "When L4_CSUM field is not supported, L4 checksum for TCP/UDP packets > can be validated by: (IP_OK && (TCP || UDP)) && (checksum == > 0xFFFF))." > Hi, Thanks Eric for your patch. Thanks Saeed for your review while I'm off-work. In IP fragments, MLX4_CQE_STATUS_IPOK is not set in cqe->status. This patch suggests falling back to CHECKSUM_COMPLETE instead of CHECKSUM_NONE. Looks good to me. > in the code we don't even consider L4_CSUM at the moment, As a future > patch, it could be a nice acceleration for the above 3 steps > condition. > Right. Good idea. I'll add it to plans. > Small comment, if we expect that cqe->checksum is NOT likely to be > 0xffff for UDP/TCP packets, maybe it is better performance wise to > move (cqe->checksum == cpu_to_be16(0xffff)) to be evaluated first in > the condition. > It _is_ likely for common TCP/UDP (no IP fragments). Let's keep it this way. >> - if (!((cqe->status & cpu_to_be16(MLX4_CQE_STATUS_IPOK)) && >> - cqe->checksum == cpu_to_be16(0xffff))) >> - goto csum_none; >> - >> l2_tunnel = (dev->hw_enc_features & NETIF_F_RXCSUM) && >> (cqe->vlan_my_qpn & cpu_to_be32(MLX4_CQE_L2_TUNNEL)); >> ip_summed = CHECKSUM_UNNECESSARY; >> -- >> 2.17.0.rc1.321.gba9d0f2565-goog >> Acked-by: Tariq Toukan <tariqt@mellanox.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments 2018-04-01 8:06 ` Tariq Toukan @ 2018-04-01 16:39 ` Eric Dumazet 2018-04-01 17:57 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Eric Dumazet @ 2018-04-01 16:39 UTC (permalink / raw) To: Tariq Toukan, Saeed Mahameed, Eric Dumazet Cc: David S . Miller, netdev, Eric Dumazet, Willem de Bruijn On 04/01/2018 01:06 AM, Tariq Toukan wrote: > > Acked-by: Tariq Toukan <tariqt@mellanox.com> Thanks Tariq and Saeed for the review ! ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments 2018-04-01 16:39 ` Eric Dumazet @ 2018-04-01 17:57 ` David Miller 0 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2018-04-01 17:57 UTC (permalink / raw) To: eric.dumazet; +Cc: tariqt, saeedm, edumazet, netdev, willemb From: Eric Dumazet <eric.dumazet@gmail.com> Date: Sun, 1 Apr 2018 09:39:38 -0700 > On 04/01/2018 01:06 AM, Tariq Toukan wrote: > >> >> Acked-by: Tariq Toukan <tariqt@mellanox.com> > > Thanks Tariq and Saeed for the review ! Applied, thanks everyone. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-01 17:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-27 21:21 [PATCH net-next] net/mlx4_en: CHECKSUM_COMPLETE support for fragments Eric Dumazet 2018-03-29 18:07 ` David Miller 2018-03-30 0:44 ` Saeed Mahameed 2018-03-30 3:32 ` Eric Dumazet 2018-03-30 0:34 ` Saeed Mahameed 2018-04-01 8:06 ` Tariq Toukan 2018-04-01 16:39 ` Eric Dumazet 2018-04-01 17:57 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox