svn commit: r314842 - head/sys/dev/xen/netfront

[Roger Pau Monn챕]

On Tue, Mar 07, 2017 at 09:52:31PM +0900, YongHyeon PYUN wrote:
> On Tue, Mar 07, 2017 at 09:18:52AM +0000, Roger Pau Monn?? wrote:
> > Author: royger
> > Date: Tue Mar  7 09:18:52 2017
> > New Revision: 314842
> > URL: https://svnweb.freebsd.org/changeset/base/314842
> > 
> > Log:
> >   xen/netfront: fix inbound packet flags for checksum offload
> >   
> >   Currently netfront is setting the flags of inbound packets with the checksum
> >   not present (offloaded) to (CSUM_IP_CHECKED | CSUM_IP_VALID | CSUM_DATA_VALID |
> >   CSUM_PSEUDO_HDR). According to the mbuf(9) man page this is not the correct
> >   combination of flags, it should instead be (CSUM_DATA_VALID |
> >   CSUM_PSEUDO_HDR).
> >   
> 
> [...]
> 
> > @@ -1192,15 +1192,17 @@ xn_rxeof(struct netfront_rxq *rxq)
> >  
> >  			m->m_pkthdr.rcvif = ifp;
> >  			if ( rx->flags & NETRXF_data_validated ) {
> > -				/* Tell the stack the checksums are okay */
> >  				/*
> > -				 * XXX this isn't necessarily the case - need to add
> > -				 * check
> > +				 * According to mbuf(9) the correct way to tell
> > +				 * the stack that the checksum of an inbound
> > +				 * packet is correct, without it actually being
> > +				 * present (because the underlying interface
> > +				 * doesn't provide it), is to set the
> > +				 * CSUM_DATA_VALID and CSUM_PSEUDO_HDR flags,
> > +				 * and the csum_data field to 0xffff.
> >  				 */
> > -
> > -				m->m_pkthdr.csum_flags |=
> > -					(CSUM_IP_CHECKED | CSUM_IP_VALID | CSUM_DATA_VALID
> > -					    | CSUM_PSEUDO_HDR);
> > +				m->m_pkthdr.csum_flags |= (CSUM_DATA_VALID
> > +				    | CSUM_PSEUDO_HDR);
> >  				m->m_pkthdr.csum_data = 0xffff;
> >  			}
> >  			if ((rx->flags & NETRXF_extra_info) != 0 &&
> > 
> 
> By chance, is there an easy way to know whether received packet is
> IPv4/IP6?  If we know received packet is IPv4, we can skip IPv4
> checksum computation too.  One of side effect of setting
> CSUM_DATA_VALID makes SCTP checksum valid for the received packet.
> I'm not sure whether this is intentional one though.

Not really, NETRXF_data_validated [0] AFAICT can be used for both IPv4 and IPv6
checksum offload. In any case, the mbuf(9) man page doesn't contain any
information about this, are there some kind of hidden flags in order to pass
this up to the stack?

> I guess if we know address family(IPv4 or IPv6), protocol (TCP, UDP
> or SCTP) and VLAN information of received packet, we can easily
> mimic all the nice offloading features of real H/W NIC.

Hm, I'm not a network expert, so I'm not familiar with all this stuff. It seems
like the netif protocol [1] has been expanded to include hash functions, and
that differentiates between IPv4/6. Sadly none of this has been integrated on
FreeBSD netfront/netback.

Roger.

[0] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/public/io/netif.h;h=ca0061410dad2797f590c5fcaf080ab5c86404b3;hb=HEAD#l902
[1] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/public/io/netif.h;h=ca0061410dad2797f590c5fcaf080ab5c86404b3;hb=HEAD#l193