On Mon, Oct 29, 2018 at 7:25 PM Eric Dumazet <eric.duma...@gmail.com> wrote: > > > > On 10/29/2018 07:21 PM, Cong Wang wrote: > > On Mon, Oct 29, 2018 at 7:14 PM Eric Dumazet <eric.duma...@gmail.com> wrote: > >> > >> Would not it be simpler to set ip_summed to CHECKSUM_NONE (no need to save > >> old_csum) ? > > > > For !CHECKSUM_COMPLETE, ip_summed should be untouched, right? > > > > If you mean only setting to CHECKSUM_NONE for CHECKSUM_COMPLETE case, > > the end result may not be simpler. > > I meant to reinstate what was there before my patch in this error case > > if (skb->ip_summed == CHECKSUM_COMPLETE) > skb->ip_summed = CHECKSUM_NONE; > > That would only be run in error (quite unlikely) path, instead of saving > old_csum in all cases.
I know your point, however, I am not sure that is a desired behavior. On failure, I think the whole skb should be restored to its previous state before entering this function, changing it to CHECKSUM_NONE on failure is inconsistent with success case.
