[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Fixed IPv6 payload lenght without jumbo option
From: |
Jason Wang |
Subject: |
Re: [PATCH] Fixed IPv6 payload lenght without jumbo option |
Date: |
Fri, 10 Apr 2020 10:28:21 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 |
On 2020/4/9 下午9:35, Andrew Melnichenko wrote:
> 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.
Right, another question.
E.g for virtio-net, we will do the following check:
if (!peer_has_vnet_hdr(n)) {
virtio_clear_feature(&features, VIRTIO_NET_F_CSUM);
virtio_clear_feature(&features, VIRTIO_NET_F_HOST_TSO4);
virtio_clear_feature(&features, VIRTIO_NET_F_HOST_TSO6);
virtio_clear_feature(&features, VIRTIO_NET_F_HOST_ECN);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_CSUM);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO4);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
}
I think we should do something similar in e1000e. But I can only see
disable_vnet parameter but not a checking of the ability of its peer.
1) when peer has vnet hdr, it supports receiving GSO packets, we don't
need software segmentation.
2) when peer does not have vnet hdr, we need to use software path
segmentation.
This means we need:
1) check peer_has_vnet_hdr() and disable_vnet in net_pkt_send() before
calling net_tx_pkt_sendv() and fallback to software segmentation
2) fix the ipv6 payload len
3) add the ipv6 software segmentation support
It would be better if we can fix all of these issue in one series.
Thanks
On Thu, Apr 9, 2020 at 6:15 AM Jason Wang <address@hidden
<mailto:address@hidden>> wrote:
On 2020/4/6 上午3:19, address@hidden <mailto:address@hidden>
wrote:
> From: Andrew Melnychenko <address@hidden
<mailto: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
<mailto: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))