Re: [dpdk-dev] [PATCH v5 1/4] vhost/lib: add vhost TX offload capabilities in vhost lib

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Thu, Nov 12, 2015 at 08:07:03PM +0800, Jijiang Liu wrote:
> Add vhost TX offload(CSUM and TSO) support capabilities in vhost lib.
> 
> Refer to feature bits in Virtual I/O Device (VIRTIO) Version 1.0 below,
> 
> VIRTIO_NET_F_CSUM (0) Device handles packets with partial checksum. This "checksum offload" is a common feature on modern network cards.
> VIRTIO_NET_F_HOST_TSO4 (11) Device can receive TSOv4.
> VIRTIO_NET_F_HOST_TSO6 (12) Device can receive TSOv6.
> 
> In order to support these features, and the following changes are added,
> 
> 1. Extend 'VHOST_SUPPORTED_FEATURES' macro to add the offload features negotiation.
> 
> 2. Dequeue TX offload: convert the fileds in virtio_net_hdr to the related fileds in mbuf.
> 
> 
> Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
...
> +static void
> +parse_ethernet(struct rte_mbuf *m, uint16_t *l4_proto, void **l4_hdr)
> +{
> +	struct ipv4_hdr *ipv4_hdr;
> +	struct ipv6_hdr *ipv6_hdr;
> +	void *l3_hdr = NULL;
> +	struct ether_hdr *eth_hdr;
> +	uint16_t ethertype;
> +
> +	eth_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
> +
> +	m->l2_len = sizeof(struct ether_hdr);
> +	ethertype = rte_be_to_cpu_16(eth_hdr->ether_type);
> +
> +	if (ethertype == ETHER_TYPE_VLAN) {
> +		struct vlan_hdr *vlan_hdr = (struct vlan_hdr *)(eth_hdr + 1);
> +
> +		m->l2_len += sizeof(struct vlan_hdr);
> +		ethertype = rte_be_to_cpu_16(vlan_hdr->eth_proto);
> +	}
> +
> +	l3_hdr = (char *)eth_hdr + m->l2_len;
> +
> +	switch (ethertype) {
> +	case ETHER_TYPE_IPv4:
> +		ipv4_hdr = (struct ipv4_hdr *)l3_hdr;
> +		*l4_proto = ipv4_hdr->next_proto_id;
> +		m->l3_len = (ipv4_hdr->version_ihl & 0x0f) * 4;
> +		*l4_hdr = (char *)l3_hdr + m->l3_len;
> +		m->ol_flags |= PKT_TX_IPV4;
> +		break;
> +	case ETHER_TYPE_IPv6:
> +		ipv6_hdr = (struct ipv6_hdr *)l3_hdr;
> +		*l4_proto = ipv6_hdr->proto;
> +		m->l3_len = sizeof(struct ipv6_hdr);
> +		*l4_hdr = (char *)l3_hdr + m->l3_len;
> +		m->ol_flags |= PKT_TX_IPV6;
> +		break;

Note that I'm still not that satisfied with putting all those kind
of calculation into vhost library.

Every application requesting TSO and CSUM offload features need
setup them, so I'm wondering _if_ we can put them into a libraray,
say lib_ether, and let the application just set few key fields
and left others to that lib.

That could leaves us from touching those chaos, such as TCP and
IP, here and there. And, that, IMO, would be a more elegant way
to leverage hardware TSO and CSUM offload features.

And I guess that might need some efforts and more discussions,
so I'm okay to left that in later versions. (Hence, I gave
my ack).

(I know little about lib_ether and DPDK hardware TSO settings,
so I could be wrong, and sorry for that if so)

	--yliu

> +	default:
> +		m->l3_len = 0;
> +		*l4_proto = 0;
> +		break;
> +	}
> +}
> +
> +static inline void __attribute__((always_inline))
> +vhost_dequeue_offload(struct virtio_net_hdr *hdr, struct rte_mbuf *m)
> +{
> +	uint16_t l4_proto = 0;
> +	void *l4_hdr = NULL;
> +	struct tcp_hdr *tcp_hdr = NULL;
> +
> +	parse_ethernet(m, &l4_proto, &l4_hdr);
> +	if (hdr->flags == VIRTIO_NET_HDR_F_NEEDS_CSUM) {
> +		if (hdr->csum_start == (m->l2_len + m->l3_len)) {
> +			switch (hdr->csum_offset) {
> +			case (offsetof(struct tcp_hdr, cksum)):
> +				if (l4_proto == IPPROTO_TCP)
> +					m->ol_flags |= PKT_TX_TCP_CKSUM;
> +				break;
> +			case (offsetof(struct udp_hdr, dgram_cksum)):
> +				if (l4_proto == IPPROTO_UDP)
> +					m->ol_flags |= PKT_TX_UDP_CKSUM;
> +				break;
> +			case (offsetof(struct sctp_hdr, cksum)):
> +				if (l4_proto == IPPROTO_SCTP)
> +					m->ol_flags |= PKT_TX_SCTP_CKSUM;
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> +		switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> +		case VIRTIO_NET_HDR_GSO_TCPV4:
> +		case VIRTIO_NET_HDR_GSO_TCPV6:
> +			tcp_hdr = (struct tcp_hdr *)l4_hdr;
> +			m->ol_flags |= PKT_TX_TCP_SEG;
> +			m->tso_segsz = hdr->gso_size;
> +			m->l4_len = (tcp_hdr->data_off & 0xf0) >> 2;
> +			break;
> +		default:
> +			RTE_LOG(WARNING, VHOST_DATA,
> +				"unsupported gso type %u.\n", hdr->gso_type);
> +			break;
> +		}
> +	}
> +}