by Ben Hutchings

[permalink] [raw]

Subject: Re: [PATCH 3.2 085/115] veth: don’t modify ip_summed; doing so treats packets with bad checksums as good.

On Wed, 2016-04-27 at 08:59 -0700, Ben Greear wrote:
> On 04/26/2016 04:02 PM, Ben Hutchings wrote:
> >
> > 3.2.80-rc1 review patch. If anyone has any objections, please let me know.
> I would be careful about this. It causes regressions when sending
> PACKET_SOCKET buffers from user-space to veth devices.
>
> There was a proposed upstream fix for the regression, but it has not gone
> into the tree as far as I know.
>
> http://www.spinics.net/lists/netdev/msg370436.html
[...]

OK, I'll drop this for now.

Ben.

--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

Attachments:

signature.asc (819.00 B)
This is a digitally signed message part

2016-04-27 21:20:22

On 04/30/2016 03:01 PM, Vijay Pandurangan wrote:
> On Sat, Apr 30, 2016 at 5:52 PM, Ben Greear <[email protected]> wrote:
>>>
>>> Good point, so if you had:
>>>
>>> eth0 <-> raw <-> user space-bridge <-> raw <-> vethA <-> veth B <->
>>> userspace-stub <->eth1
>>>
>>> and user-space hub enabled this elide flag, things would work, right?
>>> Then, it seems like what we need is a way to tell the kernel
>>> router/bridge logic to follow elide signals in packets coming from
>>> veth. I'm not sure what the best way to do this is because I'm less
>>> familiar with conventions in that part of the kernel, but assuming
>>> there's a way to do this, would it be acceptable?
>>
>>
>> You cannot receive on one veth without transmitting on the other, so
>> I think the elide csum logic can go on the raw-socket, and apply to packets
>> in the transmit-from-user-space direction. Just allowing the socket to make
>> the veth behave like it used to before this patch in question should be good
>> enough, since that worked for us for years. So, just an option to modify
>> the
>> ip_summed for pkts sent on a socket is probably sufficient.
>
> I don't think this is right. Consider:
>
> - App A sends out corrupt packets 50% of the time and discards inbound data.
> - App B doesn't care about corrupt packets and is happy to receive
> them and has some way of dealing with them (special case)
> - App C is a regular app, say nc or something.
>
> In your world, where A decides what happens to data it transmits,
> then
> A<--veth-->B and A<---wire-->B will have the same behaviour
>
> but
>
> A<-- veth --> C and A<-- wire --> C will have _different_ behaviour: C
> will behave incorrectly if it's connected over veth but correctly if
> connected with a wire. That is a bug.
>
> Since A cannot know what the app it's talking to will desire, I argue
> that both sides of a message must be opted in to this optimization.

How can you make a generic app C know how to do this? The path could be,
for instance:

eth0 <-> user-space-A <-> vethA <-> vethB <-> { kernel routing logic } <-> vethC <-> vethD <-> appC

There are no sockets on vethB, but it does need to have special behaviour to elide
csums. Even if appC is hacked to know how to twiddle some thing on it's veth port,
mucking with vethD will have no effect on vethB.

With regard to your example above, why would A corrupt packets? My guess:

1) It has bugs (so, fix the bugs, it could equally create incorrect data with proper checksums,
so just enabling checksumming adds no useful protection.)

2) It means to corrupt frames. In that case, someone must expect that C should receive incorrect
frames, otherwise why bother making App-A corrupt them in the first place?

3) You are explicitly trying to test the kernel checksum logic, so you want the kernel to
detect the bad checksum and throw away the packet. In this case, just don't set the socket
option in appA to elide checksums and the packet will be thrown away.

Any other cases you can think of?

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com