Re: [PATCH net-next 3/3] net: ipgre: add skb drop reasons to gre_rcv()

[Jakub Kicinski]

On Wed, 16 Mar 2022 14:21:24 +0800 Menglong Dong wrote:
> > I feel like gre_parse_header() is a good candidate for converting
> > to return a reason instead of errno.
> 
> Enn...isn't gre_parse_header() returning the header length? And I
> didn't find much useful reason in this function.

Ah, you're right, it returns negative error or hdr_len.
We'd need to make the reason negative, I guess that's not pretty.

What made me wonder is that it already takes a boolean for csum error
and callers don't care _which_ error gets returned. 

We can replace the csum_err output param with reason code, then?

> > >               goto drop;
> > >
> > >       if (unlikely(tpi.proto == htons(ETH_P_ERSPAN) ||
> > > -                  tpi.proto == htons(ETH_P_ERSPAN2))) {
> > > -             if (erspan_rcv(skb, &tpi, hdr_len) == PACKET_RCVD)
> > > -                     return 0;
> > > -             goto out;
> > > -     }
> > > +                  tpi.proto == htons(ETH_P_ERSPAN2)))
> > > +             ret = erspan_rcv(skb, &tpi, hdr_len);
> > > +     else
> > > +             ret = ipgre_rcv(skb, &tpi, hdr_len);  
> >
> > ipgre_rcv() OTOH may be better off taking the reason as an output
> > argument. Assuming PACKET_REJECT means NOMEM is a little fragile.  
> 
> Yeah, it seems not friendly. I think it's ok to ignore such 'NOMEM' reasons?
> Therefore, we only need to consider the PACKET_NEXT return value, and
> keep ipgre_rcv() still.