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

[Jason Wang]

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
>