On Fri, May 03, 2024 at 02:46:27PM +0200, Antonio Ojea wrote:
> On Fri, May 3, 2024 at 1:35 PM Antonio Ojea <aojea@xxxxxxxxxx> wrote:
> >
> > when the packet is processed with GSO and is SCTP it has to take into
> > account the SCTP checksum.
> >
> > Signed-off-by: Antonio Ojea <aojea@xxxxxxxxxx>
> > ---
> > net/netfilter/nfnetlink_queue.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> > index 00f4bd21c59b..428014aea396 100644
> > --- a/net/netfilter/nfnetlink_queue.c
> > +++ b/net/netfilter/nfnetlink_queue.c
> > @@ -600,6 +600,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
> > case NFQNL_COPY_PACKET:
> > if (!(queue->flags & NFQA_CFG_F_GSO) &&
> > entskb->ip_summed == CHECKSUM_PARTIAL &&
> > + (skb_csum_is_sctp(entskb) && skb_crc32c_csum_help(entskb)) &&
>
> My bad, this is wrong, it should be an OR so skb_checksum_help is
> always evaluated.
> Pablo suggested in the bugzilla to use a helper, so I'm not sure this
> is the right fix, I've tried
> to look for similar solutions to find a more consistent solution but
> I'm completely new to the
> kernel codebase so some guidance will be appreciated.
skb_crc32c_csum_help() returns 0 on success.
> - skb_checksum_help(entskb))
> + ((skb_csum_is_sctp(entskb) &&
> skb_crc32c_csum_help(entskb)) ||
> + skb_checksum_help(entskb)))
skb_crc32c_csum_help() returns 0 on success, then this calls
skb_checksum_help() which is TCP/UDP specific, which corrupts the
packet.
I think you have to move this to a helper function like:
static int nf_queue_checksum_help(struct sk_buff *entskb)
{
if (skb_csum_is_sctp(entskb))
return skb_crc32c_csum_help(entskb);
return skb_checksum_help(entskb);
}
I can see skb_crc32c_csum_help() could return -EINVAL, the fallback to
skb_checksum_help() is not wanted there in such case.
Thanks.
