Re: Checksum behaviour of bpf_redirected packets

[Alan Maguire <alan.maguire@oracle.com>]

On Mon, 1 Jun 2020, Daniel Borkmann wrote:

> On 6/1/20 7:48 PM, Alan Maguire wrote:
> > On Wed, 13 May 2020, Lorenz Bauer wrote:
> > 
> >>>> Option 1: always downgrade UNNECESSARY to NONE
> >>>> - Easiest to back port
> >>>> - The helper is safe by default
> >>>> - Performance impact unclear
> >>>> - No escape hatch for Cilium
> >>>>
> >>>> Option 2: add a flag to force CHECKSUM_NONE
> >>>> - New UAPI, can this be backported?
> >>>> - The helper isn't safe by default, needs documentation
> >>>> - Escape hatch for Cilium
> >>>>
> >>>> Option 3: downgrade to CHECKSUM_NONE, add flag to skip this
> >>>> - New UAPI, can this be backported?
> >>>> - The helper is safe by default
> >>>> - Escape hatch for Cilium (though you'd need to detect availability of
> >>>> the
> >>>>     flag somehow)
> >>>
> >>> This seems most reasonable to me; I can try and cook a proposal for
> >>> tomorrow as
> >>> potential fix. Even if we add a flag, this is still backportable to stable
> >>> (as
> >>> long as the overall patch doesn't get too complex and the backport itself
> >>> stays
> >>> compatible uapi-wise to latest kernels. We've done that before.). I happen
> >>> to
> >>> have two ixgbe NICs on some of my test machines which seem to be setting
> >>> the
> >>> CHECKSUM_UNNECESSARY, so I'll run some experiments from over here as well.
> >>
> >> Great! I'm happy to test, of course.
> > 
> > I had a go at implementing option 3 as a few colleagues ran into this
> > problem. They confirmed the fix below resolved the issue.  Daniel is
> > this  roughly what you had in mind? I can submit a patch for the bpf
> > tree if that's acceptable with the new flag. Do we need a few
> > tests though?
> 
> Coded this [0] up last week which Lorenz gave a spin as well. Originally
> wanted to
> get it out Friday night, but due to internal release stuff it got too late Fri
> night
> and didn't want to rush it at 3am anymore, so the series as fixes is going out
> tomorrow
> morning [today was public holiday in CH over here].
>

Looks great! Although I've only seen this issue arise
for cases where csum_level == 0, should we also
add "skb->csum_level = 0;" when we reset the
ip_summed value?

Feel free to add a

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>

...for the series if needed. Thanks again!

Alan

> Thanks,
> Daniel
> 
>   [0]
> https://git.kernel.org/pub/scm/linux/kernel/git/dborkman/bpf.git/log/?h=pr/adjust-csum
> 
>