[Xen-devel] Re: [1/4] [NET] back: Fix maximum fragment check

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

On Fri, Jun 30, 2006 at 02:07:16PM +0100, Keir Fraser wrote:
>
> I've merged all this already, with a few changes. I've also disabled 
> netback from advertising the feature, and also netfront from using it, 
> until we've all agreed that the inter-domain bits are sane. These 
> should end up in the public tree in an hour or two.

Thanks.

> The changes:
>  1. Pushed the gso fields into a struct inside the union. Otherwise the 
> fields overlap.

Doh! Thanks for the correction.

>  2. Changed the GSO type definitions. Currently only one type (TCPv4) 
> and the protocol type isn't really a bitmask since they are mutually 
> exclusive for a given packet. Also 'dodgy' makes no sense since netback 
> doesn't trust netfront anyway.

Agreed wrt 'dodgy' bit.

However, this really needs to be a bit mask because we'll have things
like ECN and other protocol-specific bits in future.  In fact the
upstream Linux tree has an ECN bit now.

The exclusivity will be checked by Linux (I've already submitted patches
to do this).

>  3. Renamed TXTRA->EXTRA and tx_extra -> extra_info. Looks like you 
> want to share the struct with the rx patch at some point, so making it 
> tx-specific now makes no sense. If that's not the case we can rename 
> back again.

Yes that's the plan.

>  4. I'm not sure all the error paths are now correct in netback. For 
> example, there's a call to netbk_tx_err with an end index of 0. Is that 
> right?

That was deliberate as 0 is the smallest RING_IDX.  However, now that
I look at it again there is an off-by-one bug.  I'll fix that tomorrow.

> In the latest changes I'd rather have feature-gso list the supported 
> protocols as strings (tcpv4,udpv4,etc).

Well then I might as well go back to the one int per-bit thing with
'feature-tso', 'feature-ufo', etc.  It's much easier than parsing
strings.

> Also, what happens if netfront does the following bad things:
>  1. gso.type doesn't actually match the protocol type?

This is checked by Linux due to the 'dodgy' bit.  The code isn't in
the net-gso patch yet because for now it only has one protocol.
The upstream code should have it tomorrow as we're about to add TSO6.

>  2. gso.size is set to a really small value (so that you make lots of 
> packets)?

That's OK.  TCP must deal with an MSS of 1 anyway.  The same applies to
the other protocols.

> Do we need more handling of these cases in netback? Will these be 
> safely handled in the network stack? Might we need to always work out 
> gso.type in netback for safety?

I think we've got all the bits covered now.  But if you can think of
anything else please let me know.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel