From 5d61721a621ef28d2f43fb5008afd38376be552b Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 3 Aug 2015 13:15:55 +0100 Subject: rtl8139: remove duplicate net/eth.h definitions The transmit offload features inspect Ethernet, IP, TCP, and UDP headers. Avoid redefining these net/eth.h structs. Signed-off-by: Stefan Hajnoczi Reviewed-by: Jason Wang Message-id: 1438604157-29664-2-git-send-email-stefanha@redhat.com --- hw/net/rtl8139.c | 57 +++++--------------------------------------------------- 1 file changed, 5 insertions(+), 52 deletions(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index edbb61ccf3..6de94d9cc4 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -56,6 +56,7 @@ #include "sysemu/dma.h" #include "qemu/timer.h" #include "net/net.h" +#include "net/eth.h" #include "hw/loader.h" #include "sysemu/sysemu.h" #include "qemu/iov.h" @@ -75,7 +76,6 @@ #define ETHER_ADDR_LEN 6 #define ETHER_TYPE_LEN 2 #define ETH_HLEN (ETHER_ADDR_LEN * 2 + ETHER_TYPE_LEN) -#define ETH_P_IP 0x0800 /* Internet Protocol packet */ #define ETH_P_8021Q 0x8100 /* 802.1Q VLAN Extended Header */ #define ETH_MTU 1500 @@ -1868,55 +1868,8 @@ static int rtl8139_transmit_one(RTL8139State *s, int descriptor) } /* structures and macros for task offloading */ -typedef struct ip_header -{ - uint8_t ip_ver_len; /* version and header length */ - uint8_t ip_tos; /* type of service */ - uint16_t ip_len; /* total length */ - uint16_t ip_id; /* identification */ - uint16_t ip_off; /* fragment offset field */ - uint8_t ip_ttl; /* time to live */ - uint8_t ip_p; /* protocol */ - uint16_t ip_sum; /* checksum */ - uint32_t ip_src,ip_dst; /* source and dest address */ -} ip_header; - -#define IP_HEADER_VERSION_4 4 -#define IP_HEADER_VERSION(ip) ((ip->ip_ver_len >> 4)&0xf) #define IP_HEADER_LENGTH(ip) (((ip->ip_ver_len)&0xf) << 2) -typedef struct tcp_header -{ - uint16_t th_sport; /* source port */ - uint16_t th_dport; /* destination port */ - uint32_t th_seq; /* sequence number */ - uint32_t th_ack; /* acknowledgement number */ - uint16_t th_offset_flags; /* data offset, reserved 6 bits, TCP protocol flags */ - uint16_t th_win; /* window */ - uint16_t th_sum; /* checksum */ - uint16_t th_urp; /* urgent pointer */ -} tcp_header; - -typedef struct udp_header -{ - uint16_t uh_sport; /* source port */ - uint16_t uh_dport; /* destination port */ - uint16_t uh_ulen; /* udp length */ - uint16_t uh_sum; /* udp checksum */ -} udp_header; - -typedef struct ip_pseudo_header -{ - uint32_t ip_src; - uint32_t ip_dst; - uint8_t zeros; - uint8_t ip_proto; - uint16_t ip_payload; -} ip_pseudo_header; - -#define IP_PROTO_TCP 6 -#define IP_PROTO_UDP 17 - #define TCP_HEADER_DATA_OFFSET(tcp) (((be16_to_cpu(tcp->th_offset_flags) >> 12)&0xf) << 2) #define TCP_FLAGS_ONLY(flags) ((flags)&0x3f) #define TCP_HEADER_FLAGS(tcp) TCP_FLAGS_ONLY(be16_to_cpu(tcp->th_offset_flags)) @@ -2151,12 +2104,12 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) DPRINTF("+++ C+ mode offloaded task checksum\n"); /* Large enough for Ethernet and IP headers? */ - if (saved_size < ETH_HLEN + sizeof(ip_header)) { + if (saved_size < ETH_HLEN + sizeof(struct ip_header)) { goto skip_offload; } /* ip packet header */ - ip_header *ip = NULL; + struct ip_header *ip = NULL; int hlen = 0; uint8_t ip_protocol = 0; uint16_t ip_data_len = 0; @@ -2176,7 +2129,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) eth_payload_data = saved_buffer + ETH_HLEN; eth_payload_len = saved_size - ETH_HLEN; - ip = (ip_header*)eth_payload_data; + ip = (struct ip_header*)eth_payload_data; if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) { DPRINTF("+++ C+ mode packet has bad IP version %d " @@ -2186,7 +2139,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) } hlen = IP_HEADER_LENGTH(ip); - if (hlen < sizeof(ip_header) || hlen > eth_payload_len) { + if (hlen < sizeof(struct ip_header) || hlen > eth_payload_len) { goto skip_offload; } -- cgit v1.2.3 From 1bf11332c4770e2750247733c713a4e771047282 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 3 Aug 2015 13:15:56 +0100 Subject: rtl8139: use net/eth.h macros instead of custom macros Eliminate the following "custom" macros since they are just duplicates of net/eth.h macros under a different name: ETHER_ADDR_LEN -> ETH_ALEN ETH_P_8021Q -> ETH_P_VLAN IP_HEADER_LENGTH -> IP_HDR_GET_LEN TCP_FLAG_FIN -> TH_FIN TCP_FLAG_PUSH -> TH_PUSH Signed-off-by: Stefan Hajnoczi Reviewed-by: Jason Wang Message-id: 1438604157-29664-3-git-send-email-stefanha@redhat.com --- hw/net/rtl8139.c | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 6de94d9cc4..36be22b99c 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -73,10 +73,8 @@ #define MOD2(input, size) \ ( ( input ) & ( size - 1 ) ) -#define ETHER_ADDR_LEN 6 #define ETHER_TYPE_LEN 2 -#define ETH_HLEN (ETHER_ADDR_LEN * 2 + ETHER_TYPE_LEN) -#define ETH_P_8021Q 0x8100 /* 802.1Q VLAN Extended Header */ +#define ETH_HLEN (ETH_ALEN * 2 + ETHER_TYPE_LEN) #define ETH_MTU 1500 #define VLAN_TCI_LEN 2 @@ -1016,8 +1014,8 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t /* write VLAN info to descriptor variables. */ if (s->CpCmd & CPlusRxVLAN && be16_to_cpup((uint16_t *) - &buf[ETHER_ADDR_LEN * 2]) == ETH_P_8021Q) { - dot1q_buf = &buf[ETHER_ADDR_LEN * 2]; + &buf[ETH_ALEN * 2]) == ETH_P_VLAN) { + dot1q_buf = &buf[ETH_ALEN * 2]; size -= VLAN_HLEN; /* if too small buffer, use the tailroom added duing expansion */ if (size < MIN_BUF_SIZE) { @@ -1058,10 +1056,10 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t /* receive/copy to target memory */ if (dot1q_buf) { - pci_dma_write(d, rx_addr, buf, 2 * ETHER_ADDR_LEN); - pci_dma_write(d, rx_addr + 2 * ETHER_ADDR_LEN, - buf + 2 * ETHER_ADDR_LEN + VLAN_HLEN, - size - 2 * ETHER_ADDR_LEN); + pci_dma_write(d, rx_addr, buf, 2 * ETH_ALEN); + pci_dma_write(d, rx_addr + 2 * ETH_ALEN, + buf + 2 * ETH_ALEN + VLAN_HLEN, + size - 2 * ETH_ALEN); } else { pci_dma_write(d, rx_addr, buf, size); } @@ -1783,12 +1781,12 @@ static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, return; } - if (dot1q_buf && size >= ETHER_ADDR_LEN * 2) { + if (dot1q_buf && size >= ETH_ALEN * 2) { iov = (struct iovec[3]) { - { .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 }, + { .iov_base = buf, .iov_len = ETH_ALEN * 2 }, { .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN }, - { .iov_base = buf + ETHER_ADDR_LEN * 2, - .iov_len = size - ETHER_ADDR_LEN * 2 }, + { .iov_base = buf + ETH_ALEN * 2, + .iov_len = size - ETH_ALEN * 2 }, }; memcpy(vlan_iov, iov, sizeof(vlan_iov)); @@ -1868,17 +1866,12 @@ static int rtl8139_transmit_one(RTL8139State *s, int descriptor) } /* structures and macros for task offloading */ -#define IP_HEADER_LENGTH(ip) (((ip->ip_ver_len)&0xf) << 2) - #define TCP_HEADER_DATA_OFFSET(tcp) (((be16_to_cpu(tcp->th_offset_flags) >> 12)&0xf) << 2) #define TCP_FLAGS_ONLY(flags) ((flags)&0x3f) #define TCP_HEADER_FLAGS(tcp) TCP_FLAGS_ONLY(be16_to_cpu(tcp->th_offset_flags)) #define TCP_HEADER_CLEAR_FLAGS(tcp, off) ((tcp)->th_offset_flags &= cpu_to_be16(~TCP_FLAGS_ONLY(off))) -#define TCP_FLAG_FIN 0x01 -#define TCP_FLAG_PUSH 0x08 - /* produces ones' complement sum of data */ static uint16_t ones_complement_sum(uint8_t *data, size_t len) { @@ -2087,7 +2080,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) bswap16(txdw1 & CP_TX_VLAN_TAG_MASK)); dot1q_buffer = (uint16_t *) dot1q_buffer_space; - dot1q_buffer[0] = cpu_to_be16(ETH_P_8021Q); + dot1q_buffer[0] = cpu_to_be16(ETH_P_VLAN); /* BE + le_to_cpu() + ~cpu_to_le()~ = BE */ dot1q_buffer[1] = cpu_to_le16(txdw1 & CP_TX_VLAN_TAG_MASK); } else { @@ -2138,7 +2131,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) goto skip_offload; } - hlen = IP_HEADER_LENGTH(ip); + hlen = IP_HDR_GET_LEN(ip); if (hlen < sizeof(struct ip_header) || hlen > eth_payload_len) { goto skip_offload; } @@ -2240,7 +2233,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) /* keep PUSH and FIN flags only for the last frame */ if (!is_last_frame) { - TCP_HEADER_CLEAR_FLAGS(p_tcp_hdr, TCP_FLAG_PUSH|TCP_FLAG_FIN); + TCP_HEADER_CLEAR_FLAGS(p_tcp_hdr, TH_PUSH | TH_FIN); } /* recalculate TCP checksum */ -- cgit v1.2.3 From 26c0114d3f69c3accaf83d56ff1d850bd0213b58 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 3 Aug 2015 13:15:57 +0100 Subject: rtl8139: use ldl/stl wrapper for unaligned 32-bit access The tx offload feature accesses a 16-bit aligned TCP header struct. The 32-bit fields must be accessed using ldl/stl wrappers since some host architectures fault on unaligned access. Suggested-by: Peter Maydell Signed-off-by: Stefan Hajnoczi Reviewed-by: Jason Wang Message-id: 1438604157-29664-4-git-send-email-stefanha@redhat.com --- hw/net/rtl8139.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 36be22b99c..366d1b59bf 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2118,7 +2118,11 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) DPRINTF("+++ C+ mode has IP packet\n"); - /* not aligned */ + /* Note on memory alignment: eth_payload_data is 16-bit aligned + * since saved_buffer is allocated with g_malloc() and ETH_HLEN is + * even. 32-bit accesses must use ldl/stl wrappers to avoid + * unaligned accesses. + */ eth_payload_data = saved_buffer + ETH_HLEN; eth_payload_len = saved_size - ETH_HLEN; @@ -2215,7 +2219,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) } DPRINTF("+++ C+ mode TSO TCP seqno %08x\n", - be32_to_cpu(p_tcp_hdr->th_seq)); + ldl_be_p(&p_tcp_hdr->th_seq)); /* add 4 TCP pseudoheader fields */ /* copy IP source and destination fields */ @@ -2271,7 +2275,8 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) 0, (uint8_t *) dot1q_buffer); /* add transferred count to TCP sequence number */ - p_tcp_hdr->th_seq = cpu_to_be32(chunk_size + be32_to_cpu(p_tcp_hdr->th_seq)); + stl_be_p(&p_tcp_hdr->th_seq, + chunk_size + ldl_be_p(&p_tcp_hdr->th_seq)); ++send_count; } -- cgit v1.2.3 From fabdcd3392f16fc666b1d04fc1bbe5f1dbbf10a4 Mon Sep 17 00:00:00 2001 From: Vladislav Yasevich Date: Tue, 1 Sep 2015 11:26:45 -0400 Subject: rtl8139: Fix receive buffer overflow check rtl8139_do_receive() tries to check for the overflow condition by making sure that packet_size + 8 does not exceed the available buffer space. The issue here is that RxBuffAddr, used to calculate available buffer space, is aligned to a a 4 byte boundry after every update. So it is possible that every packet ends up being slightly padded when written to the receive buffer. This padding is not taken into account when checking for overflow and we may end up missing the overflow condition can causing buffer overwrite. This patch takes alignment into consideration when checking for overflow condition. Signed-off-by: Vladislav Yasevich Reviewed-by: Jason Wang Message-id: 1441121206-6997-2-git-send-email-vyasevic@redhat.com Signed-off-by: Stefan Hajnoczi --- hw/net/rtl8139.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 366d1b59bf..960580b910 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -1146,7 +1146,9 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t /* if receiver buffer is empty then avail == 0 */ - if (avail != 0 && size + 8 >= avail) +#define RX_ALIGN(x) (((x) + 3) & ~0x3) + + if (avail != 0 && RX_ALIGN(size + 8) >= avail) { DPRINTF("rx overflow: rx buffer length %d head 0x%04x " "read 0x%04x === available 0x%04x need 0x%04x\n", @@ -1174,7 +1176,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t rtl8139_write_buffer(s, (uint8_t *)&val, 4); /* correct buffer write pointer */ - s->RxBufAddr = MOD2((s->RxBufAddr + 3) & ~0x3, s->RxBufferSize); + s->RxBufAddr = MOD2(RX_ALIGN(s->RxBufAddr), s->RxBufferSize); /* now we can signal we have received something */ -- cgit v1.2.3 From 26c4e7ca72d970d120f0f51244bc8d37458512a0 Mon Sep 17 00:00:00 2001 From: Vladislav Yasevich Date: Tue, 1 Sep 2015 11:26:46 -0400 Subject: rtl8139: Do not consume the packet during overflow in standard mode. When operation in standard mode, we currently return the size of packet during buffer overflow. This consumes the overflow packet. Return 0 instead so we can re-process the overflow packet when we have room. This fixes issues with lost/dropped fragments of large messages. Signed-off-by: Vladislav Yasevich Reviewed-by: Jason Wang Message-id: 1441121206-6997-3-git-send-email-vyasevic@redhat.com Signed-off-by: Stefan Hajnoczi --- hw/net/rtl8139.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 960580b910..fb2c55ce0b 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -1157,7 +1157,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t s->IntrStatus |= RxOverflow; ++s->RxMissed; rtl8139_update_irq(s); - return size_; + return 0; } packet_header |= RxStatusOK; -- cgit v1.2.3 From 2734a20b8161831ba68c9166014e00522599d1e2 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 1 Jul 2015 10:26:27 +0800 Subject: vmxnet3: Drop net_vmxnet3_info.can_receive Commit 6e99c63 ("net/socket: Drop net_socket_can_send") changed the semantics around .can_receive for sockets to now require the device to flush queued pkts when transitioning to a .can_receive=true state. But it's OK to drop incoming packets when the link is not active. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- hw/net/vmxnet3.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 071feebf15..04159c8222 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -1988,7 +1988,6 @@ static void vmxnet3_set_link_status(NetClientState *nc) static NetClientInfo net_vmxnet3_info = { .type = NET_CLIENT_OPTIONS_KIND_NIC, .size = sizeof(NICState), - .can_receive = vmxnet3_can_receive, .receive = vmxnet3_receive, .link_status_changed = vmxnet3_set_link_status, }; -- cgit v1.2.3 From c5a93780453e6da919287c17e873c843544ef2a3 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 1 Jul 2015 13:25:05 +0800 Subject: ne2000: Drop ne2000_can_receive ne2000_receive already checks the same conditions and drops the packet if it's not ready, removing the .can_receive callback avoids the necessity to add explicit flushes when the conditions turn true (which is required by the new semantics of .can_receive since 6e99c63 "net/socket: Drop net_socket_can_send"). Plus the "return 1" if E8390_STOP is also suspicious. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- hw/net/ne2000-isa.c | 1 - hw/net/ne2000.c | 10 ---------- hw/net/ne2000.h | 1 - 3 files changed, 12 deletions(-) diff --git a/hw/net/ne2000-isa.c b/hw/net/ne2000-isa.c index 17e7199f70..18b064463a 100644 --- a/hw/net/ne2000-isa.c +++ b/hw/net/ne2000-isa.c @@ -44,7 +44,6 @@ typedef struct ISANE2000State { static NetClientInfo net_ne2000_isa_info = { .type = NET_CLIENT_OPTIONS_KIND_NIC, .size = sizeof(NICState), - .can_receive = ne2000_can_receive, .receive = ne2000_receive, }; diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c index 3492db3663..53c704ad41 100644 --- a/hw/net/ne2000.c +++ b/hw/net/ne2000.c @@ -165,15 +165,6 @@ static int ne2000_buffer_full(NE2000State *s) return 0; } -int ne2000_can_receive(NetClientState *nc) -{ - NE2000State *s = qemu_get_nic_opaque(nc); - - if (s->cmd & E8390_STOP) - return 1; - return !ne2000_buffer_full(s); -} - #define MIN_BUF_SIZE 60 ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_) @@ -705,7 +696,6 @@ void ne2000_setup_io(NE2000State *s, DeviceState *dev, unsigned size) static NetClientInfo net_ne2000_info = { .type = NET_CLIENT_OPTIONS_KIND_NIC, .size = sizeof(NICState), - .can_receive = ne2000_can_receive, .receive = ne2000_receive, }; diff --git a/hw/net/ne2000.h b/hw/net/ne2000.h index e500306aac..d022b28fc2 100644 --- a/hw/net/ne2000.h +++ b/hw/net/ne2000.h @@ -34,7 +34,6 @@ typedef struct NE2000State { void ne2000_setup_io(NE2000State *s, DeviceState *dev, unsigned size); extern const VMStateDescription vmstate_ne2000; void ne2000_reset(NE2000State *s); -int ne2000_can_receive(NetClientState *nc); ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_); #endif -- cgit v1.2.3