The following changes since commit e81eb5e6d108008445821e4f891fb9563016c71b:

Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2021-01-23 22:34:21 +0000)

for you to fetch changes up to f574633529926697ced51b6865e5c50bbb78bf1b:

net: checksum: Introduce fine control over checksum type (2021-01-25 17:04:56 +0800)

Bin Meng (1):

net: checksum: Introduce fine control over checksum type

Guishan Qin (1):

net: checksum: Add IP header checksum calculation

Markus Armbruster (1):

net: Fix handling of id in netdev_add and netdev_del

Markus Carlstedt (1):

net: checksum: Skip fragmented IP packets

hw/net/allwinner-sun8i-emac.c | 2 +-

hw/net/cadence_gem.c | 2 +-

hw/net/fsl_etsec/rings.c | 18 +++++++++---------

hw/net/ftgmac100.c | 13 ++++++++++++-

hw/net/imx_fec.c | 20 ++++++++------------

hw/net/virtio-net.c | 2 +-

hw/net/xen_nic.c | 2 +-

include/net/checksum.h | 7 ++++++-

net/checksum.c | 24 +++++++++++++++++++++---

net/filter-rewriter.c | 4 ++--

net/net.c | 20 ++++++++++++++++++--

11 files changed, 80 insertions(+), 34 deletions(-)

From: Markus Armbruster <armbru@redhat.com>

CLI -netdev accumulates in option group "netdev".

Before commit 08712fcb85 "net: Track netdevs in NetClientState rather

than QemuOpt", netdev_add added to the option group, and netdev_del

removed from it, both HMP and QMP. Thus, every netdev had a

corresponding QemuOpts in this option group.

Commit 08712fcb85 dropped this for QMP netdev_add and both netdev_del.

Now a netdev has a corresponding QemuOpts only when it was created

with CLI or HMP. Two issues:

* QMP and HMP netdev_del can leave QemuOpts behind, breaking HMP

netdev_add. Reproducer:

$ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio

QEMU 5.1.92 monitor - type 'help' for more information

(qemu) netdev_add user,id=net0

(qemu) info network

net0: index=0,type=user,net=10.0.2.0,restrict=off

(qemu) netdev_del net0

(qemu) info network

(qemu) netdev_add user,id=net0

upstream-qemu: Duplicate ID 'net0' for netdev

Try "help netdev_add" for more information

Fix by restoring the QemuOpts deletion in qmp_netdev_del(), but with

a guard, because the QemuOpts need not exist.

* QMP netdev_add loses its "no duplicate ID" check. Reproducer:

$ qemu-system-x86_64 -S -display none -qmp stdio

{"QMP": {"version": {"qemu": {"micro": 92, "minor": 1, "major": 5}, "package": "v5.2.0-rc2-1-g02c1f0142c"}, "capabilities": ["oob"]}}

{"execute": "qmp_capabilities"}

{"return": {}}

{"execute": "netdev_add", "arguments": {"type": "user", "id":"net0"}}

{"return": {}}

{"execute": "netdev_add", "arguments": {"type": "user", "id":"net0"}}

{"return": {}}

Fix by adding a duplicate ID check to net_client_init1() to replace

the lost one. The check is redundant for callers where QemuOpts

still checks, i.e. for CLI and HMP.

Reported-by: Andrew Melnichenko <andrew@daynix.com>

Fixes: 08712fcb851034228b61f75bd922863a984a4f60

Cc: qemu-stable@nongnu.org

Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

net/net.c | 20 ++++++++++++++++++--

1 file changed, 18 insertions(+), 2 deletions(-)

@@ -XXX,XX +XXX,XX @@ static int (* const net_client_init_fun[NET_CLIENT_DRIVER__MAX])(

static int net_client_init1(const Netdev *netdev, bool is_netdev, Error **errp)

{

NetClientState *peer = NULL;

+ NetClientState *nc;

if (is_netdev) {

if (netdev->type == NET_CLIENT_DRIVER_NIC ||

@@ -XXX,XX +XXX,XX @@ static int net_client_init1(const Netdev *netdev, bool is_netdev, Error **errp)

From: Markus Carlstedt <markus.carlstedt@windriver.com>

To calculate the TCP/UDP checksum we need the whole datagram. Unless

the hardware has some logic to collect all fragments before sending

the whole datagram first, it can only be done by the network stack,

which is normally the case for the NICs we have seen so far.

Skip these fragmented IP packets to avoid checksum corruption.

Signed-off-by: Markus Carlstedt <markus.carlstedt@windriver.com>

Signed-off-by: Bin Meng <bin.meng@windriver.com>

net/checksum.c | 4 ++++

1 file changed, 4 insertions(+)

diff --git a/net/checksum.c b/net/checksum.c

--- a/net/checksum.c

+++ b/net/checksum.c

@@ -XXX,XX +XXX,XX @@ void net_checksum_calculate(uint8_t *data, int length)

return; /* not IPv4 */

}

+ if (IP4_IS_FRAGMENT(ip)) {

+ return; /* a fragmented IP packet */

+ }

+

ip_len = lduw_be_p(&ip->ip_len);

/* Last, check that we have enough data for the all IP frame */

From: Guishan Qin <guishan.qin@windriver.com>

At present net_checksum_calculate() only calculates TCP/UDP checksum

in an IP packet, but assumes the IP header checksum to be provided

by the software, e.g.: Linux kernel always calculates the IP header

checksum. However this might not always be the case, e.g.: for an IP

checksum offload enabled stack like VxWorks, the IP header checksum

can be zero.

This adds the checksum calculation of the IP header.

Signed-off-by: Guishan Qin <guishan.qin@windriver.com>

Signed-off-by: Yabing Liu <yabing.liu@windriver.com>

Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

net/checksum.c | 8 ++++++--

1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/checksum.c b/net/checksum.c

--- a/net/checksum.c

+++ b/net/checksum.c

@@ -XXX,XX +XXX,XX @@ void net_checksum_calculate(uint8_t *data, int length)

{

int mac_hdr_len, ip_len;

struct ip_header *ip;

+ uint16_t csum;

/*

* Note: We cannot assume "data" is aligned, so the all code uses

@@ -XXX,XX +XXX,XX @@ void net_checksum_calculate(uint8_t *data, int length)

return; /* not IPv4 */

}

+ /* Calculate IP checksum */

+ stw_he_p(&ip->ip_sum, 0);

+ csum = net_raw_checksum((uint8_t *)ip, IP_HDR_GET_LEN(ip));

+ stw_be_p(&ip->ip_sum, csum);

+

if (IP4_IS_FRAGMENT(ip)) {

return; /* a fragmented IP packet */

}

@@ -XXX,XX +XXX,XX @@ void net_checksum_calculate(uint8_t *data, int length)

switch (ip->ip_p) {

case IP_PROTO_TCP:

{

- uint16_t csum;

tcp_header *tcp = (tcp_header *)(ip + 1);

if (ip_len < sizeof(tcp_header)) {

@@ -XXX,XX +XXX,XX @@ void net_checksum_calculate(uint8_t *data, int length)

}

case IP_PROTO_UDP:

{

- uint16_t csum;

udp_header *udp = (udp_header *)(ip + 1);

if (ip_len < sizeof(udp_header)) {

From: Bin Meng <bin.meng@windriver.com>

At present net_checksum_calculate() blindly calculates all types of

checksums (IP, TCP, UDP). Some NICs may have a per type setting in

their BDs to control what checksum should be offloaded. To support

such hardware behavior, introduce a 'csum_flag' parameter to the

net_checksum_calculate() API to allow fine control over what type

checksum is calculated.

Existing users of this API are updated accordingly.

Signed-off-by: Bin Meng <bin.meng@windriver.com>

hw/net/allwinner-sun8i-emac.c | 2 +-

hw/net/cadence_gem.c | 2 +-

hw/net/fsl_etsec/rings.c | 18 +++++++++---------

hw/net/ftgmac100.c | 13 ++++++++++++-

hw/net/imx_fec.c | 20 ++++++++------------

hw/net/virtio-net.c | 2 +-

hw/net/xen_nic.c | 2 +-

include/net/checksum.h | 7 ++++++-

net/checksum.c | 18 ++++++++++++++----

net/filter-rewriter.c | 4 ++--

10 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/net/allwinner-sun8i-emac.c

+++ b/hw/net/allwinner-sun8i-emac.c

@@ -XXX,XX +XXX,XX @@ static void allwinner_sun8i_emac_transmit(AwSun8iEmacState *s)

/* After the last descriptor, send the packet */

if (desc.status2 & TX_DESC_STATUS2_LAST_DESC) {

if (desc.status2 & TX_DESC_STATUS2_CHECKSUM_MASK) {

- net_checksum_calculate(packet_buf, packet_bytes);

+ net_checksum_calculate(packet_buf, packet_bytes, CSUM_ALL);

}

qemu_send_packet(nc, packet_buf, packet_bytes);

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/net/cadence_gem.c

+++ b/hw/net/cadence_gem.c

@@ -XXX,XX +XXX,XX @@ static void gem_transmit(CadenceGEMState *s)

/* Is checksum offload enabled? */

if (s->regs[GEM_DMACFG] & GEM_DMACFG_TXCSUM_OFFL) {

- net_checksum_calculate(s->tx_packet, total_bytes);

+ net_checksum_calculate(s->tx_packet, total_bytes, CSUM_ALL);

}

/* Update MAC statistics */

diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/net/fsl_etsec/rings.c

+++ b/hw/net/fsl_etsec/rings.c

@@ -XXX,XX +XXX,XX @@ static void process_tx_fcb(eTSEC *etsec)

uint8_t *l3_header = etsec->tx_buffer + 8 + l3_header_offset;

/* L4 header */

uint8_t *l4_header = l3_header + l4_header_offset;

+ int csum = 0;

/* if packet is IP4 and IP checksum is requested */

if (flags & FCB_TX_IP && flags & FCB_TX_CIP) {

- /* do IP4 checksum (TODO This function does TCP/UDP checksum

- * but not sure if it also does IP4 checksum.) */

- net_checksum_calculate(etsec->tx_buffer + 8,

- etsec->tx_buffer_len - 8);

+ csum |= CSUM_IP;

}

/* TODO Check the correct usage of the PHCS field of the FCB in case the NPH

* flag is on */

@@ -XXX,XX +XXX,XX @@ static void process_tx_fcb(eTSEC *etsec)

/* if checksum is requested */

if (flags & FCB_TX_CTU) {

/* do UDP checksum */

-

- net_checksum_calculate(etsec->tx_buffer + 8,

- etsec->tx_buffer_len - 8);

+ csum |= CSUM_UDP;

} else {

/* set checksum field to 0 */

l4_header[6] = 0;

@@ -XXX,XX +XXX,XX @@ static void process_tx_fcb(eTSEC *etsec)

}

} else if (flags & FCB_TX_CTU) { /* if TCP and checksum is requested */

/* do TCP checksum */

- net_checksum_calculate(etsec->tx_buffer + 8,

- etsec->tx_buffer_len - 8);

+ csum |= CSUM_TCP;

}

}

+

+ if (csum) {

+ net_checksum_calculate(etsec->tx_buffer + 8,

+ etsec->tx_buffer_len - 8, csum);

+ }

}

static void process_tx_bd(eTSEC *etsec,

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/net/ftgmac100.c

+++ b/hw/net/ftgmac100.c

@@ -XXX,XX +XXX,XX @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,

ptr += len;

frame_size += len;

if (bd.des0 & FTGMAC100_TXDES0_LTS) {

+ int csum = 0;

/* Check for VLAN */

if (flags & FTGMAC100_TXDES1_INS_VLANTAG &&

@@ -XXX,XX +XXX,XX @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,

}

if (flags & FTGMAC100_TXDES1_IP_CHKSUM) {

- net_checksum_calculate(s->frame, frame_size);

+ csum |= CSUM_IP;

}

+ if (flags & FTGMAC100_TXDES1_TCP_CHKSUM) {

+ csum |= CSUM_TCP;

+ }

+ if (flags & FTGMAC100_TXDES1_UDP_CHKSUM) {

+ csum |= CSUM_UDP;

+ }

+ if (csum) {

+ net_checksum_calculate(s->frame, frame_size, csum);

+ }

+

/* Last buffer in frame. */

qemu_send_packet(qemu_get_queue(s->nic), s->frame, frame_size);

ptr = s->frame;

diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/net/imx_fec.c

+++ b/hw/net/imx_fec.c

@@ -XXX,XX +XXX,XX @@ static void imx_enet_do_tx(IMXFECState *s, uint32_t index)

ptr += len;

frame_size += len;

if (bd.flags & ENET_BD_L) {

+ int csum = 0;

+

if (bd.option & ENET_BD_PINS) {

- struct ip_header *ip_hd = PKT_GET_IP_HDR(s->frame);

- if (IP_HEADER_VERSION(ip_hd) == 4) {

- net_checksum_calculate(s->frame, frame_size);

- }

+ csum |= (CSUM_TCP | CSUM_UDP);

}

if (bd.option & ENET_BD_IINS) {

- struct ip_header *ip_hd = PKT_GET_IP_HDR(s->frame);

- /* We compute checksum only for IPv4 frames */

- if (IP_HEADER_VERSION(ip_hd) == 4) {

- uint16_t csum;

- ip_hd->ip_sum = 0;

- csum = net_raw_checksum((uint8_t *)ip_hd, sizeof(*ip_hd));

- ip_hd->ip_sum = cpu_to_be16(csum);

- }

+ csum |= CSUM_IP;

+ }

+ if (csum) {

+ net_checksum_calculate(s->frame, frame_size, csum);

}

+

/* Last buffer in frame. */

qemu_send_packet(qemu_get_queue(s->nic), s->frame, frame_size);

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/net/virtio-net.c

+++ b/hw/net/virtio-net.c

@@ -XXX,XX +XXX,XX @@ static void work_around_broken_dhclient(struct virtio_net_hdr *hdr,

(buf[12] == 0x08 && buf[13] == 0x00) && /* ethertype == IPv4 */

(buf[23] == 17) && /* ip.protocol == UDP */

(buf[34] == 0 && buf[35] == 67)) { /* udp.srcport == bootps */

- net_checksum_calculate(buf, size);

+ net_checksum_calculate(buf, size, CSUM_UDP);

hdr->flags &= ~VIRTIO_NET_HDR_F_NEEDS_CSUM;

}

}

diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/net/xen_nic.c

+++ b/hw/net/xen_nic.c

@@ -XXX,XX +XXX,XX @@ static void net_tx_packets(struct XenNetDev *netdev)

tmpbuf = g_malloc(XC_PAGE_SIZE);

}

memcpy(tmpbuf, page + txreq.offset, txreq.size);

- net_checksum_calculate(tmpbuf, txreq.size);

+ net_checksum_calculate(tmpbuf, txreq.size, CSUM_ALL);

qemu_send_packet(qemu_get_queue(netdev->nic), tmpbuf,

txreq.size);

} else {

diff --git a/include/net/checksum.h b/include/net/checksum.h

index XXXXXXX..XXXXXXX 100644

--- a/include/net/checksum.h

+++ b/include/net/checksum.h

#include "qemu/bswap.h"

struct iovec;

+#define CSUM_IP 0x01

+#define CSUM_TCP 0x02

+#define CSUM_UDP 0x04

+#define CSUM_ALL (CSUM_IP | CSUM_TCP | CSUM_UDP)

+

uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq);

uint16_t net_checksum_finish(uint32_t sum);

uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto,

uint8_t *addrs, uint8_t *buf);

-void net_checksum_calculate(uint8_t *data, int length);

+void net_checksum_calculate(uint8_t *data, int length, int csum_flag);

static inline uint32_t

net_checksum_add(int len, uint8_t *buf)

diff --git a/net/checksum.c b/net/checksum.c

index XXXXXXX..XXXXXXX 100644

--- a/net/checksum.c

+++ b/net/checksum.c

@@ -XXX,XX +XXX,XX @@ uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto,

return net_checksum_finish(sum);

}

-void net_checksum_calculate(uint8_t *data, int length)

+void net_checksum_calculate(uint8_t *data, int length, int csum_flag)

int mac_hdr_len, ip_len;

struct ip_header *ip;

@@ -XXX,XX +XXX,XX @@ void net_checksum_calculate(uint8_t *data, int length)

}

/* Calculate IP checksum */

- stw_he_p(&ip->ip_sum, 0);

- csum = net_raw_checksum((uint8_t *)ip, IP_HDR_GET_LEN(ip));

- stw_be_p(&ip->ip_sum, csum);

+ if (csum_flag & CSUM_IP) {

+ stw_he_p(&ip->ip_sum, 0);

+ csum = net_raw_checksum((uint8_t *)ip, IP_HDR_GET_LEN(ip));

+ stw_be_p(&ip->ip_sum, csum);

if (IP4_IS_FRAGMENT(ip)) {

return; /* a fragmented IP packet */

@@ -XXX,XX +XXX,XX @@ void net_checksum_calculate(uint8_t *data, int length)

switch (ip->ip_p) {

case IP_PROTO_TCP:

{

+ if (!(csum_flag & CSUM_TCP)) {

+ return;

+

tcp_header *tcp = (tcp_header *)(ip + 1);

if (ip_len < sizeof(tcp_header)) {

@@ -XXX,XX +XXX,XX @@ void net_checksum_calculate(uint8_t *data, int length)

}

case IP_PROTO_UDP:

{

+ if (!(csum_flag & CSUM_UDP)) {

+ return;

+ }

+

udp_header *udp = (udp_header *)(ip + 1);

if (ip_len < sizeof(udp_header)) {

diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c

index XXXXXXX..XXXXXXX 100644

--- a/net/filter-rewriter.c

+++ b/net/filter-rewriter.c

@@ -XXX,XX +XXX,XX @@ static int handle_primary_tcp_pkt(RewriterState *rf,

tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset);

net_checksum_calculate((uint8_t *)pkt->data + pkt->vnet_hdr_len,

- pkt->size - pkt->vnet_hdr_len);

+ pkt->size - pkt->vnet_hdr_len, CSUM_TCP);

/*

@@ -XXX,XX +XXX,XX @@ static int handle_secondary_tcp_pkt(RewriterState *rf,

tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset);

net_checksum_calculate((uint8_t *)pkt->data + pkt->vnet_hdr_len,

- pkt->size - pkt->vnet_hdr_len);

+ pkt->size - pkt->vnet_hdr_len, CSUM_TCP);

}

}