Re: [ovs-dev] [PATCH v4 6/6] netdev-dpdk: Refactor tunnel checksum offloading.

2024-06-06 Thread Kevin Traynor
On 05/06/2024 19:16, Ilya Maximets wrote:
> On 5/31/24 16:10, Kevin Traynor wrote:
>> On 30/05/2024 14:10, David Marchand wrote:
>>> All informations required for checksum offloading can be deducted by
>>
>> nit:  "All information required for checksum offloading can be deduced
>> by" can update on applying, assuming no more revs are needed.
>>
>>> already tracked dp_packet l3_ofs, l4_ofs, inner_l3_ofs and inner_l4_ofs
>>> fields.
>>> Remove DPDK specific l[2-4]_len from generic OVS code.
>>>
>>> netdev-dpdk code then fills mbuf specifics step by step:
>>> - outer_l2_len and outer_l3_len are needed for tunneling (and below
>>>   features),
>>> - l2_len and l3_len are needed for IP and L4 checksum (and below features),
>>> - l4_len and tso_segsz are needed when doing TSO,
>>>
>>> Signed-off-by: David Marchand 
>>> ---
>>>  lib/dp-packet.h | 37 --
>>>  lib/netdev-dpdk.c   | 35 ++---
>>>  lib/netdev-native-tnl.c | 50 +
>>>  3 files changed, 27 insertions(+), 95 deletions(-)
>>
>> Acked-by: Kevin Traynor 
> 
> Thanks, David and Kevin!
> 
> I generally like the direction of this patch set, especially the
> cleanup of the generic tunnel code.
> 
> I didn't test it with a real hardware nor I re-checked the math,
> so will not Ack it, but it looks good to me otherwise, and I think
> we should backport the whole thing to at least branch-3.3 as well.
> 
> Best regards, Ilya Maximets.
> 

Thanks David, Jun and Ilya. Series applied and backported to branch-3.3.

Kevin.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v4 6/6] netdev-dpdk: Refactor tunnel checksum offloading.

2024-06-05 Thread Ilya Maximets
On 5/31/24 16:10, Kevin Traynor wrote:
> On 30/05/2024 14:10, David Marchand wrote:
>> All informations required for checksum offloading can be deducted by
> 
> nit:  "All information required for checksum offloading can be deduced
> by" can update on applying, assuming no more revs are needed.
> 
>> already tracked dp_packet l3_ofs, l4_ofs, inner_l3_ofs and inner_l4_ofs
>> fields.
>> Remove DPDK specific l[2-4]_len from generic OVS code.
>>
>> netdev-dpdk code then fills mbuf specifics step by step:
>> - outer_l2_len and outer_l3_len are needed for tunneling (and below
>>   features),
>> - l2_len and l3_len are needed for IP and L4 checksum (and below features),
>> - l4_len and tso_segsz are needed when doing TSO,
>>
>> Signed-off-by: David Marchand 
>> ---
>>  lib/dp-packet.h | 37 --
>>  lib/netdev-dpdk.c   | 35 ++---
>>  lib/netdev-native-tnl.c | 50 +
>>  3 files changed, 27 insertions(+), 95 deletions(-)
> 
> Acked-by: Kevin Traynor 

Thanks, David and Kevin!

I generally like the direction of this patch set, especially the
cleanup of the generic tunnel code.

I didn't test it with a real hardware nor I re-checked the math,
so will not Ack it, but it looks good to me otherwise, and I think
we should backport the whole thing to at least branch-3.3 as well.

Best regards, Ilya Maximets.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v4 6/6] netdev-dpdk: Refactor tunnel checksum offloading.

2024-05-31 Thread Kevin Traynor
On 30/05/2024 14:10, David Marchand wrote:
> All informations required for checksum offloading can be deducted by

nit:  "All information required for checksum offloading can be deduced
by" can update on applying, assuming no more revs are needed.

> already tracked dp_packet l3_ofs, l4_ofs, inner_l3_ofs and inner_l4_ofs
> fields.
> Remove DPDK specific l[2-4]_len from generic OVS code.
> 
> netdev-dpdk code then fills mbuf specifics step by step:
> - outer_l2_len and outer_l3_len are needed for tunneling (and below
>   features),
> - l2_len and l3_len are needed for IP and L4 checksum (and below features),
> - l4_len and tso_segsz are needed when doing TSO,
> 
> Signed-off-by: David Marchand 
> ---
>  lib/dp-packet.h | 37 --
>  lib/netdev-dpdk.c   | 35 ++---
>  lib/netdev-native-tnl.c | 50 +
>  3 files changed, 27 insertions(+), 95 deletions(-)

Acked-by: Kevin Traynor 



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v4 6/6] netdev-dpdk: Refactor tunnel checksum offloading.

2024-05-30 Thread David Marchand
All informations required for checksum offloading can be deducted by
already tracked dp_packet l3_ofs, l4_ofs, inner_l3_ofs and inner_l4_ofs
fields.
Remove DPDK specific l[2-4]_len from generic OVS code.

netdev-dpdk code then fills mbuf specifics step by step:
- outer_l2_len and outer_l3_len are needed for tunneling (and below
  features),
- l2_len and l3_len are needed for IP and L4 checksum (and below features),
- l4_len and tso_segsz are needed when doing TSO,

Signed-off-by: David Marchand 
---
 lib/dp-packet.h | 37 --
 lib/netdev-dpdk.c   | 35 ++---
 lib/netdev-native-tnl.c | 50 +
 3 files changed, 27 insertions(+), 95 deletions(-)

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 3622764c47..a75b1c5cdb 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -604,25 +604,6 @@ dp_packet_get_nd_payload(const struct dp_packet *b)
 }
 
 #ifdef DPDK_NETDEV
-static inline void
-dp_packet_set_l2_len(struct dp_packet *b, size_t l2_len)
-{
-b->mbuf.l2_len = l2_len;
-}
-
-static inline void
-dp_packet_set_l3_len(struct dp_packet *b, size_t l3_len)
-{
-b->mbuf.l3_len = l3_len;
-}
-
-static inline void
-dp_packet_set_l4_len(struct dp_packet *b, size_t l4_len)
-{
-b->mbuf.l4_len = l4_len;
-}
-
-
 static inline uint64_t *
 dp_packet_ol_flags_ptr(const struct dp_packet *b)
 {
@@ -642,24 +623,6 @@ dp_packet_flow_mark_ptr(const struct dp_packet *b)
 }
 
 #else
-static inline void
-dp_packet_set_l2_len(struct dp_packet *b OVS_UNUSED, size_t l2_len OVS_UNUSED)
-{
-/* There is no implementation. */
-}
-
-static inline void
-dp_packet_set_l3_len(struct dp_packet *b OVS_UNUSED, size_t l3_len OVS_UNUSED)
-{
-/* There is no implementation. */
-}
-
-static inline void
-dp_packet_set_l4_len(struct dp_packet *b OVS_UNUSED, size_t l4_len OVS_UNUSED)
-{
-/* There is no implementation. */
-}
-
 static inline uint32_t *
 dp_packet_ol_flags_ptr(const struct dp_packet *b)
 {
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index bda3fa94b6..0fa37d5145 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2583,6 +2583,9 @@ static bool
 netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, struct rte_mbuf *mbuf)
 {
 struct dp_packet *pkt = CONTAINER_OF(mbuf, struct dp_packet, mbuf);
+void *l2;
+void *l3;
+void *l4;
 
 const uint64_t all_inner_requests = (RTE_MBUF_F_TX_IP_CKSUM |
  RTE_MBUF_F_TX_L4_MASK |
@@ -2612,11 +2615,6 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
struct rte_mbuf *mbuf)
 return true;
 }
 
-ovs_assert(dp_packet_l4(pkt));
-
-/* If packet is vxlan or geneve tunnel packet, calculate outer
- * l2 len and outer l3 len. Inner l2/l3/l4 len are calculated
- * before. */
 const uint64_t tunnel_type = mbuf->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK;
 if (OVS_UNLIKELY(tunnel_type &&
  tunnel_type != RTE_MBUF_F_TX_TUNNEL_GENEVE &&
@@ -2634,6 +2632,11 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
struct rte_mbuf *mbuf)
  (char *) dp_packet_eth(pkt);
 mbuf->outer_l3_len = (char *) dp_packet_l4(pkt) -
  (char *) dp_packet_l3(pkt);
+
+/* Inner L2 length must account for the tunnel header length. */
+l2 = dp_packet_l4(pkt);
+l3 = dp_packet_inner_l3(pkt);
+l4 = dp_packet_inner_l4(pkt);
 } else {
 /* If no outer offloading is requested, clear outer marks. */
 mbuf->ol_flags &= ~all_outer_marks;
@@ -2641,8 +2644,9 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
struct rte_mbuf *mbuf)
 mbuf->outer_l3_len = 0;
 
 /* Skip outer headers. */
-mbuf->l2_len += (char *) dp_packet_l4(pkt) -
-(char *) dp_packet_eth(pkt);
+l2 = dp_packet_eth(pkt);
+l3 = dp_packet_inner_l3(pkt);
+l4 = dp_packet_inner_l4(pkt);
 }
 } else {
 if (tunnel_type) {
@@ -2662,22 +2666,27 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, 
struct rte_mbuf *mbuf)
 }
 mbuf->outer_l2_len = 0;
 mbuf->outer_l3_len = 0;
-mbuf->l2_len = (char *) dp_packet_l3(pkt) -
-   (char *) dp_packet_eth(pkt);
-mbuf->l3_len = (char *) dp_packet_l4(pkt) -
-   (char *) dp_packet_l3(pkt);
+
+l2 = dp_packet_eth(pkt);
+l3 = dp_packet_l3(pkt);
+l4 = dp_packet_l4(pkt);
 }
 
+ovs_assert(l4);
+
+mbuf->l2_len = (char *) l3 - (char *) l2;
+mbuf->l3_len = (char *) l4 - (char *) l3;
+
 if (mbuf->ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
-struct tcp_header *th = dp_packet_l4(pkt);
+struct tcp_header *th = l4;
 uint16_t link_tso_segsz;
 int hdr_len;
 
+mbuf->l4_len = TCP_OFFSET(th->tcp_ctl) * 4;