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(+)
From: Andrew Melnychenko <andrew@daynix.com>
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 <andrew@daynix.com>
---
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);
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))
--
2.24.1
On 4/5/20 2:19 PM, andrew@daynix.com wrote: > From: Andrew Melnychenko <andrew@daynix.com> In the subject: s/lenght/length/ > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1708065 > e1000e driver doesn't sets 'plen' field for IPv6 for big packets s/sets/set the/ > if TSO is enabled. Jumbo option isn't added yet, until > qemu supports packets greater than 64K. > > Signed-off-by: Andrew Melnychenko <andrew@daynix.com> > --- -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 2020/4/6 上午3:19, andrew@daynix.com wrote:
> From: Andrew Melnychenko <andrew@daynix.com>
>
> 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 <andrew@daynix.com>
> ---
> 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))
> 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 <jasowang@redhat.com> wrote:
>
> On 2020/4/6 上午3:19, andrew@daynix.com wrote:
> > From: Andrew Melnychenko <andrew@daynix.com>
> >
> > 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 <andrew@daynix.com>
> > ---
> > 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))
>
>
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 <jasowang@redhat.com
> <mailto:jasowang@redhat.com>> wrote:
>
>
> On 2020/4/6 上午3:19, andrew@daynix.com <mailto:andrew@daynix.com>
> wrote:
> > From: Andrew Melnychenko <andrew@daynix.com
> <mailto:andrew@daynix.com>>
> >
> > 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 <andrew@daynix.com
> <mailto:andrew@daynix.com>>
> > ---
> > 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))
>
Ok, later - I'll prepare a new patch with length fix, segmentation and
checks.
For now, the current patch can be discarded.
On Fri, Apr 10, 2020 at 5:28 AM Jason Wang <jasowang@redhat.com> wrote:
>
> 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 <jasowang@redhat.com
> > <mailto:jasowang@redhat.com>> wrote:
> >
> >
> > On 2020/4/6 上午3:19, andrew@daynix.com <mailto:andrew@daynix.com>
> > wrote:
> > > From: Andrew Melnychenko <andrew@daynix.com
> > <mailto:andrew@daynix.com>>
> > >
> > > 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 <andrew@daynix.com
> > <mailto:andrew@daynix.com>>
> > > ---
> > > 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))
> >
>
>
© 2016 - 2024 Red Hat, Inc.