Re: [PATCH] Fixed IPv6 payload lenght without jumbo option

[Andrew Melnichenko]

> Do we support ipv6 segmentation then?

No, but - if the backend supports big frames there is an issue that IPv6 plen doesn't have valid value.

Actually, It's a good idea to add IPv6 fragmentation - I would do it later.

On Thu, Apr 9, 2020 at 6:15 AM Jason Wang <address@hidden> wrote:

On 2020/4/6 上午3:19, address@hidden wrote:
> From: Andrew Melnychenko <address@hidden>
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1708065
> e1000e driver doesn't sets 'plen' field for IPv6 for big packets
> if TSO is enabled. Jumbo option isn't added yet, until
> qemu supports packets greater than 64K.
>
> Signed-off-by: Andrew Melnychenko <address@hidden>
> ---
>   hw/net/e1000e_core.c |  1 +
>   hw/net/net_tx_pkt.c  | 31 +++++++++++++++++++++++++++++++
>   hw/net/net_tx_pkt.h  |  7 +++++++
>   include/net/eth.h    |  1 +
>   4 files changed, 40 insertions(+)
>
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index d5676871fa..a1ec55598b 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -656,6 +656,7 @@ e1000e_tx_pkt_send(E1000ECore *core, struct e1000e_tx *tx, int queue_index)
>       NetClientState *queue = qemu_get_subqueue(core->owner_nic, target_queue);
>   
>       e1000e_setup_tx_offloads(core, tx);
> +    net_tx_pkt_fix_ip6_payload_len(tx->tx_pkt);


A question here:

I don't see any code that set ip6_plen during
net_tx_pkt_do_sw_fragmentation(). This is described in 7.3.6.2.1 and
7.3.6.2.2 in the datasheet.

And:

1) eth_setup_ip4_fragmentation() only deal with ipv4

2) eth_fix_ip4_checksum() assumes a ipv4 header

Do we support ipv6 segmentation then?

Thanks


>   
>       net_tx_pkt_dump(tx->tx_pkt);
>   
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index 162f802dd7..b05d554ac3 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -635,3 +635,34 @@ bool net_tx_pkt_send_loopback(struct NetTxPkt *pkt, NetClientState *nc)
>   
>       return res;
>   }
> +
> +void net_tx_pkt_fix_ip6_payload_len(struct NetTxPkt *pkt)
> +{
> +    /*
> +     * If ipv6 payload length field is 0 - then there should be Hop-by-Hop
> +     * option for packets greater than 65,535.
> +     * For packets with payload less than 65,535: fix 'plen' field.
> +     * For now, qemu drops every packet with size greater 64K
> +     * (see net_tx_pkt_send()) so, there is no reason to add jumbo option to ip6
> +     * hop-by-hop extension if it's missed
> +     */
> +
> +    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->payload_len <= ETH_MAX_IP_DGRAM_LEN) {
> +                ip6->ip6_plen = htons(pkt->payload_len);
> +            }
> +            /*
> +             * TODO: if qemu would support >64K packets
> +             * add jumbo option for packets greater then 65,535 bytes
> +             */
> +        }
> +    }
> +}
> diff --git a/hw/net/net_tx_pkt.h b/hw/net/net_tx_pkt.h
> index 212ecc62fc..912d56ef13 100644
> --- a/hw/net/net_tx_pkt.h
> +++ b/hw/net/net_tx_pkt.h
> @@ -187,4 +187,11 @@ bool net_tx_pkt_parse(struct NetTxPkt *pkt);
>   */
>   bool net_tx_pkt_has_fragments(struct NetTxPkt *pkt);
>   
> +/**
> + * Fix IPv6 'plen' field.
> + *
> + * @pkt            packet
> + */
> +void net_tx_pkt_fix_ip6_payload_len(struct NetTxPkt *pkt);
> +
>   #endif
> diff --git a/include/net/eth.h b/include/net/eth.h
> index 7f45c678e7..0671be6916 100644
> --- a/include/net/eth.h
> +++ b/include/net/eth.h
> @@ -186,6 +186,7 @@ struct tcp_hdr {
>   
>   #define ip6_nxt      ip6_ctlun.ip6_un1.ip6_un1_nxt
>   #define ip6_ecn_acc  ip6_ctlun.ip6_un3.ip6_un3_ecn
> +#define ip6_plen     ip6_ctlun.ip6_un1.ip6_un1_plen
>   
>   #define PKT_GET_ETH_HDR(p)        \
>       ((struct eth_header *)(p))