[PATCH for 8.0] hw/net/net_tx_pkt: Align l3_hdr

Akihiko Odaki posted 1 patch 1 year ago
hw/net/net_tx_pkt.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
[PATCH for 8.0] hw/net/net_tx_pkt: Align l3_hdr
Posted by Akihiko Odaki 1 year ago
Align the l3_hdr member of NetTxPkt by defining it as a union of
ip_header, ip6_header, and an array of octets.

Fixes: e263cd49c7 ("Packet abstraction for VMWARE network devices")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1544
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/net/net_tx_pkt.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index efe80b1a80..8dc8568ba2 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -43,7 +43,11 @@ struct NetTxPkt {
     struct iovec *vec;
 
     uint8_t l2_hdr[ETH_MAX_L2_HDR_LEN];
-    uint8_t l3_hdr[ETH_MAX_IP_DGRAM_LEN];
+    union {
+        struct ip_header ip;
+        struct ip6_header ip6;
+        uint8_t octets[ETH_MAX_IP_DGRAM_LEN];
+    } l3_hdr;
 
     uint32_t payload_len;
 
@@ -89,16 +93,14 @@ void net_tx_pkt_update_ip_hdr_checksum(struct NetTxPkt *pkt)
 {
     uint16_t csum;
     assert(pkt);
-    struct ip_header *ip_hdr;
-    ip_hdr = pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_base;
 
-    ip_hdr->ip_len = cpu_to_be16(pkt->payload_len +
+    pkt->l3_hdr.ip.ip_len = cpu_to_be16(pkt->payload_len +
         pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_len);
 
-    ip_hdr->ip_sum = 0;
-    csum = net_raw_checksum((uint8_t *)ip_hdr,
+    pkt->l3_hdr.ip.ip_sum = 0;
+    csum = net_raw_checksum(pkt->l3_hdr.octets,
         pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_len);
-    ip_hdr->ip_sum = cpu_to_be16(csum);
+    pkt->l3_hdr.ip.ip_sum = cpu_to_be16(csum);
 }
 
 void net_tx_pkt_update_ip_checksums(struct NetTxPkt *pkt)
@@ -832,15 +834,14 @@ void net_tx_pkt_fix_ip6_payload_len(struct NetTxPkt *pkt)
 {
     struct iovec *l2 = &pkt->vec[NET_TX_PKT_L2HDR_FRAG];
     if (eth_get_l3_proto(l2, 1, l2->iov_len) == ETH_P_IPV6) {
-        struct ip6_header *ip6 = (struct ip6_header *) pkt->l3_hdr;
         /*
          * TODO: if qemu would support >64K packets - add jumbo option check
          * something like that:
          * 'if (ip6->ip6_plen == 0 && !has_jumbo_option(ip6)) {'
          */
-        if (ip6->ip6_plen == 0) {
+        if (pkt->l3_hdr.ip6.ip6_plen == 0) {
             if (pkt->payload_len <= ETH_MAX_IP_DGRAM_LEN) {
-                ip6->ip6_plen = htons(pkt->payload_len);
+                pkt->l3_hdr.ip6.ip6_plen = htons(pkt->payload_len);
             }
             /*
              * TODO: if qemu would support >64K packets
-- 
2.39.2
Re: [PATCH for 8.0] hw/net/net_tx_pkt: Align l3_hdr
Posted by Akihiko Odaki 1 year ago
Hi Jason,

I have sent some patches for igb and packet abstractions. They are fixes 
intended to be included in 8.0 so please have a look at them.

Regards,
Akihiko Odaki

On 2023/03/16 21:26, Akihiko Odaki wrote:
> Align the l3_hdr member of NetTxPkt by defining it as a union of
> ip_header, ip6_header, and an array of octets.
> 
> Fixes: e263cd49c7 ("Packet abstraction for VMWARE network devices")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1544
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   hw/net/net_tx_pkt.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index efe80b1a80..8dc8568ba2 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -43,7 +43,11 @@ struct NetTxPkt {
>       struct iovec *vec;
>   
>       uint8_t l2_hdr[ETH_MAX_L2_HDR_LEN];
> -    uint8_t l3_hdr[ETH_MAX_IP_DGRAM_LEN];
> +    union {
> +        struct ip_header ip;
> +        struct ip6_header ip6;
> +        uint8_t octets[ETH_MAX_IP_DGRAM_LEN];
> +    } l3_hdr;
>   
>       uint32_t payload_len;
>   
> @@ -89,16 +93,14 @@ void net_tx_pkt_update_ip_hdr_checksum(struct NetTxPkt *pkt)
>   {
>       uint16_t csum;
>       assert(pkt);
> -    struct ip_header *ip_hdr;
> -    ip_hdr = pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_base;
>   
> -    ip_hdr->ip_len = cpu_to_be16(pkt->payload_len +
> +    pkt->l3_hdr.ip.ip_len = cpu_to_be16(pkt->payload_len +
>           pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_len);
>   
> -    ip_hdr->ip_sum = 0;
> -    csum = net_raw_checksum((uint8_t *)ip_hdr,
> +    pkt->l3_hdr.ip.ip_sum = 0;
> +    csum = net_raw_checksum(pkt->l3_hdr.octets,
>           pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_len);
> -    ip_hdr->ip_sum = cpu_to_be16(csum);
> +    pkt->l3_hdr.ip.ip_sum = cpu_to_be16(csum);
>   }
>   
>   void net_tx_pkt_update_ip_checksums(struct NetTxPkt *pkt)
> @@ -832,15 +834,14 @@ void net_tx_pkt_fix_ip6_payload_len(struct NetTxPkt *pkt)
>   {
>       struct iovec *l2 = &pkt->vec[NET_TX_PKT_L2HDR_FRAG];
>       if (eth_get_l3_proto(l2, 1, l2->iov_len) == ETH_P_IPV6) {
> -        struct ip6_header *ip6 = (struct ip6_header *) pkt->l3_hdr;
>           /*
>            * TODO: if qemu would support >64K packets - add jumbo option check
>            * something like that:
>            * 'if (ip6->ip6_plen == 0 && !has_jumbo_option(ip6)) {'
>            */
> -        if (ip6->ip6_plen == 0) {
> +        if (pkt->l3_hdr.ip6.ip6_plen == 0) {
>               if (pkt->payload_len <= ETH_MAX_IP_DGRAM_LEN) {
> -                ip6->ip6_plen = htons(pkt->payload_len);
> +                pkt->l3_hdr.ip6.ip6_plen = htons(pkt->payload_len);
>               }
>               /*
>                * TODO: if qemu would support >64K packets
Re: [PATCH for 8.0] hw/net/net_tx_pkt: Align l3_hdr
Posted by Jason Wang 1 year ago
On Thu, Mar 23, 2023 at 4:53 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Hi Jason,
>
> I have sent some patches for igb and packet abstractions. They are fixes
> intended to be included in 8.0 so please have a look at them.

I gave some comments and If possible please post them with a series
then Sriram can rebase on top.

Thanks

>
> Regards,
> Akihiko Odaki
>
> On 2023/03/16 21:26, Akihiko Odaki wrote:
> > Align the l3_hdr member of NetTxPkt by defining it as a union of
> > ip_header, ip6_header, and an array of octets.
> >
> > Fixes: e263cd49c7 ("Packet abstraction for VMWARE network devices")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1544
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > ---
> >   hw/net/net_tx_pkt.c | 21 +++++++++++----------
> >   1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> > index efe80b1a80..8dc8568ba2 100644
> > --- a/hw/net/net_tx_pkt.c
> > +++ b/hw/net/net_tx_pkt.c
> > @@ -43,7 +43,11 @@ struct NetTxPkt {
> >       struct iovec *vec;
> >
> >       uint8_t l2_hdr[ETH_MAX_L2_HDR_LEN];
> > -    uint8_t l3_hdr[ETH_MAX_IP_DGRAM_LEN];
> > +    union {
> > +        struct ip_header ip;
> > +        struct ip6_header ip6;
> > +        uint8_t octets[ETH_MAX_IP_DGRAM_LEN];
> > +    } l3_hdr;
> >
> >       uint32_t payload_len;
> >
> > @@ -89,16 +93,14 @@ void net_tx_pkt_update_ip_hdr_checksum(struct NetTxPkt *pkt)
> >   {
> >       uint16_t csum;
> >       assert(pkt);
> > -    struct ip_header *ip_hdr;
> > -    ip_hdr = pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_base;
> >
> > -    ip_hdr->ip_len = cpu_to_be16(pkt->payload_len +
> > +    pkt->l3_hdr.ip.ip_len = cpu_to_be16(pkt->payload_len +
> >           pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_len);
> >
> > -    ip_hdr->ip_sum = 0;
> > -    csum = net_raw_checksum((uint8_t *)ip_hdr,
> > +    pkt->l3_hdr.ip.ip_sum = 0;
> > +    csum = net_raw_checksum(pkt->l3_hdr.octets,
> >           pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_len);
> > -    ip_hdr->ip_sum = cpu_to_be16(csum);
> > +    pkt->l3_hdr.ip.ip_sum = cpu_to_be16(csum);
> >   }
> >
> >   void net_tx_pkt_update_ip_checksums(struct NetTxPkt *pkt)
> > @@ -832,15 +834,14 @@ void net_tx_pkt_fix_ip6_payload_len(struct NetTxPkt *pkt)
> >   {
> >       struct iovec *l2 = &pkt->vec[NET_TX_PKT_L2HDR_FRAG];
> >       if (eth_get_l3_proto(l2, 1, l2->iov_len) == ETH_P_IPV6) {
> > -        struct ip6_header *ip6 = (struct ip6_header *) pkt->l3_hdr;
> >           /*
> >            * TODO: if qemu would support >64K packets - add jumbo option check
> >            * something like that:
> >            * 'if (ip6->ip6_plen == 0 && !has_jumbo_option(ip6)) {'
> >            */
> > -        if (ip6->ip6_plen == 0) {
> > +        if (pkt->l3_hdr.ip6.ip6_plen == 0) {
> >               if (pkt->payload_len <= ETH_MAX_IP_DGRAM_LEN) {
> > -                ip6->ip6_plen = htons(pkt->payload_len);
> > +                pkt->l3_hdr.ip6.ip6_plen = htons(pkt->payload_len);
> >               }
> >               /*
> >                * TODO: if qemu would support >64K packets
>