diff mbox series

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

Message ID 20240130221439.24297-2-mkp@redhat.com
State Changes Requested
Headers show
Series [ovs-dev,v2,1/4] dp-packet: Validate correct offset for L4 inner size. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/intel-ovs-compilation fail test: fail
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Mike Pattrick Jan. 30, 2024, 10:14 p.m. UTC 
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 <mkp@redhat.com>
---
v2: Corrected formatting, and just calculate checksum up front
---
 lib/bfd.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

Comments

David Marchand Jan. 31, 2024, 2:47 p.m. UTC | #1 
On Tue, Jan 30, 2024 at 11:15 PM Mike Pattrick <mkp@redhat.com> 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 <mkp@redhat.com>

Reviewed-by: David Marchand <david.marchand@redhat.com>
diff mbox series

Patch

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;