On Tue, Apr 11, 2017 at 10:13 AM, Edward Cree <ec...@solarflare.com> wrote:
> On 11/04/17 17:46, Tom Herbert wrote:
>> On Tue, Apr 11, 2017 at 8:55 AM, Edward Cree <ec...@solarflare.com> wrote:
>>> The counter-argument, of course, is that if the XDP program edits fields
>>> that are protected by an Internet checksum (which in practice usually
>>> means anything but the Ethernet header) and then fixes up the checksums
>>> itself, we will edit our diff value twice just to conclude that diff==0.
>>> And maybe we are willing to trust root with the ability to lie to the
>>> kernel about incoming packets' checksum_complete values.
>>>
>> Works for modifying IPv4 or when we are expressly doing a checksum
>> neutral operation, but that doesn't cover the general case. Suppose we
>> simply want to pop an IPv6 header in IPv6/IPv6 encpasulation (there is
>> some push in IETF for expanded use of IPIP tunneling with IPv6 to deal
>> with the inserting EH into a packet problem). In that case the bits
>> being chucked won't sum to zero and they're probably not also the same
>> so the trick for ILA won't work so we need something to rectify the
>> checksum if we're doing XDP_PASS for instance.
> Oh, I agree that if we don't do the rewriting we have to have some way
> for XDP to output a diff. I agree with Daniel that a helper function
> seems the obvious way to get this information out. My "counter-
> argument" was about "user does it" vs. "verifier does it".
>> As far as trust is concerned I don't think that is a major issue. The
>> XDP program can already modify the packet in arbitrary ways so if the
>> checksum is messed up this likely results in unnecessarily dropping
>> packets that actually a good checksum (as opposed to allowing packets
>> with bad checksum to pass).
> I wasn't thinking so much of trust in the security sense (I agree that's
> not much of an argument since XDP programs are privileged already), but
> rather as a reliability thing; it maybe being an easy thing for root to
> get wrong ("hey, this bit only edits Ethernet headers so I don't have
> to fix any checksums, job done. Why is my network broken?")
> So I think we'd have to assume that any program that _doesn't_ call the
> helper function has invalidated any checksum_complete value we had; if
> all the program's changes were balanced, it can call update_csum(0).
>> Note that this only applies to
>> checksum_complete, if we were to allow XDP program to return
>> checksum_unnecessary for instance then it's more a leap of faith that
>> things are always correct.
> Speaking of checksum_unnecessary, it might still be useful for the
> verifier to tell us whether the program contains any writes, since if
> so any drivers using checksum_unnecessary will have to clear it when
> calling XDP or else do conversion to checksum_complete beforehand. (We
> at sfc will probably take the latter path.) But this can be elided if
> the XDP program is known not to do writes or header adjustments,
> potentially speeding things up for the pure drop case.
> (Also we wouldn't require such a program to call update_csum for the
> checksum_complete case, of course.)
>
I don't think we need to do anything special for checksum_unnecessary
other than what we need for checksum_complete. For instance, if we do
return the checksum diff from XDP program then a non-zero value would
be sufficient to invalidate the checksum_unnecessary. Of course, it's
likely this is overkill since in many cases the changes wouldn't
affect checksum_unnecessary setting, but trying to unravel that to
preserve checksum_unnecessary would be a mess. checksum_unnecessary is
considered deprecated anyway!
Tom
> -Ed
