Re: [ovs-dev] [PATCH v2 2/4] bfd: Set proper offsets and flags in BFD packets.

2024-01-31 Thread David Marchand
On Tue, Jan 30, 2024 at 11:15 PM Mike Pattrick  wrote:
>
> Previously the BFD packet creation code did not appropriately set
> offsets or flags. This contributed to issues involving encapsulation and
> the TSO code.

I noted that apart from fixing the offsets / flags used to checksum
offloading, this patch also fixes the packet_type used by other
dp_packet helpers.
I see nothing fixed on that later topic though.


>
> Fixes: ccc096898c46 ("bfd: Implement Bidirectional Forwarding Detection.")
> Signed-off-by: Mike Pattrick 

Reviewed-by: David Marchand 


-- 
David Marchand

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


[ovs-dev] [PATCH v2 2/4] bfd: Set proper offsets and flags in BFD packets.

2024-01-30 Thread Mike Pattrick
Previously the BFD packet creation code did not appropriately set
offsets or flags. This contributed to issues involving encapsulation and
the TSO code.

Fixes: ccc096898c46 ("bfd: Implement Bidirectional Forwarding Detection.")
Signed-off-by: Mike Pattrick 
---
v2: Corrected formatting, and just calculate checksum up front
---
 lib/bfd.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/lib/bfd.c b/lib/bfd.c
index 9698576d0..9af258917 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -586,7 +586,6 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
 {
 long long int min_tx, min_rx;
 struct udp_header *udp;
-struct eth_header *eth;
 struct ip_header *ip;
 struct msg *msg;
 
@@ -605,15 +604,13 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
  * set. */
 ovs_assert(!(bfd->flags & FLAG_POLL) || !(bfd->flags & FLAG_FINAL));
 
-dp_packet_reserve(p, 2); /* Properly align after the ethernet header. */
-eth = dp_packet_put_uninit(p, sizeof *eth);
-eth->eth_src = eth_addr_is_zero(bfd->local_eth_src)
-? eth_src : bfd->local_eth_src;
-eth->eth_dst = eth_addr_is_zero(bfd->local_eth_dst)
-? eth_addr_bfd : bfd->local_eth_dst;
-eth->eth_type = htons(ETH_TYPE_IP);
+ip = eth_compose(p,
+ eth_addr_is_zero(bfd->local_eth_dst)
+ ? eth_addr_bfd : bfd->local_eth_dst,
+ eth_addr_is_zero(bfd->local_eth_src)
+ ? eth_src : bfd->local_eth_src,
+ ETH_TYPE_IP, sizeof *ip + sizeof *udp + sizeof *msg);
 
-ip = dp_packet_put_zeros(p, sizeof *ip);
 ip->ip_ihl_ver = IP_IHL_VER(5, 4);
 ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
 ip->ip_ttl = MAXTTL;
@@ -621,15 +618,17 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
 ip->ip_proto = IPPROTO_UDP;
 put_16aligned_be32(&ip->ip_src, bfd->ip_src);
 put_16aligned_be32(&ip->ip_dst, bfd->ip_dst);
-/* Checksum has already been zeroed by put_zeros call. */
+/* Checksum has already been zeroed by eth_compose call. */
 ip->ip_csum = csum(ip, sizeof *ip);
+dp_packet_set_l4(p, ip + 1);
 
-udp = dp_packet_put_zeros(p, sizeof *udp);
+udp = dp_packet_l4(p);
 udp->udp_src = htons(bfd->udp_src);
 udp->udp_dst = htons(BFD_DEST_PORT);
 udp->udp_len = htons(sizeof *udp + sizeof *msg);
+/* Checksum already zero from eth_compose. */
 
-msg = dp_packet_put_uninit(p, sizeof *msg);
+msg = (struct msg *)(udp + 1);
 msg->vers_diag = (BFD_VERSION << 5) | bfd->diag;
 msg->flags = (bfd->state & STATE_MASK) | bfd->flags;
 
-- 
2.39.3

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