lkml.org 
[lkml]   [2022]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH net-next 3/3] net: ipgre: add skb drop reasons to gre_rcv()
    On Mon, 14 Mar 2022 21:33:12 +0800 menglong8.dong@gmail.com wrote:
    > From: Menglong Dong <imagedong@tencent.com>
    >
    > Replace kfree_skb() used in gre_rcv() with kfree_skb_reason(). With
    > previous patch, we can tell that no tunnel device is found when
    > PACKET_NEXT is returned by erspan_rcv() or ipgre_rcv().
    >
    > In this commit, following new drop reasons are added:
    >
    > SKB_DROP_REASON_GRE_CSUM
    > SKB_DROP_REASON_GRE_NOTUNNEL
    >
    > Reviewed-by: Hao Peng <flyingpeng@tencent.com>
    > Reviewed-by: Biao Jiang <benbjiang@tencent.com>
    > Signed-off-by: Menglong Dong <imagedong@tencent.com>
    > ---
    > include/linux/skbuff.h | 2 ++
    > include/trace/events/skb.h | 2 ++
    > net/ipv4/ip_gre.c | 28 ++++++++++++++++++----------
    > 3 files changed, 22 insertions(+), 10 deletions(-)
    >
    > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
    > index 5edb704af5bb..4f5e58e717ee 100644
    > --- a/include/linux/skbuff.h
    > +++ b/include/linux/skbuff.h
    > @@ -448,6 +448,8 @@ enum skb_drop_reason {
    > SKB_DROP_REASON_GRE_NOHANDLER, /* no handler found (version not
    > * supported?)
    > */
    > + SKB_DROP_REASON_GRE_CSUM, /* GRE csum error */
    > + SKB_DROP_REASON_GRE_NOTUNNEL, /* no tunnel device found */
    > SKB_DROP_REASON_MAX,
    > };
    >
    > diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
    > index f2bcffdc4bae..e8f95c96cf9d 100644
    > --- a/include/trace/events/skb.h
    > +++ b/include/trace/events/skb.h
    > @@ -63,6 +63,8 @@
    > EM(SKB_DROP_REASON_TAP_TXFILTER, TAP_TXFILTER) \
    > EM(SKB_DROP_REASON_GRE_VERSION, GRE_VERSION) \
    > EM(SKB_DROP_REASON_GRE_NOHANDLER, GRE_NOHANDLER) \
    > + EM(SKB_DROP_REASON_GRE_CSUM, GRE_CSUM) \
    > + EM(SKB_DROP_REASON_GRE_NOTUNNEL, GRE_NOTUNNEL) \
    > EMe(SKB_DROP_REASON_MAX, MAX)
    >
    > #undef EM
    > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
    > index b1579d8374fd..b989239e4abc 100644
    > --- a/net/ipv4/ip_gre.c
    > +++ b/net/ipv4/ip_gre.c
    > @@ -421,9 +421,10 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi,
    >
    > static int gre_rcv(struct sk_buff *skb)
    > {
    > + enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED;
    > struct tnl_ptk_info tpi;
    > bool csum_err = false;
    > - int hdr_len;
    > + int hdr_len, ret;
    >
    > #ifdef CONFIG_NET_IPGRE_BROADCAST
    > if (ipv4_is_multicast(ip_hdr(skb)->daddr)) {
    > @@ -438,19 +439,26 @@ static int gre_rcv(struct sk_buff *skb)

    I feel like gre_parse_header() is a good candidate for converting
    to return a reason instead of errno.


    > 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.

    >
    > - if (ipgre_rcv(skb, &tpi, hdr_len) == PACKET_RCVD)
    > + switch (ret) {
    > + case PACKET_NEXT:
    > + reason = SKB_DROP_REASON_GRE_NOTUNNEL;
    > + break;
    > + case PACKET_RCVD:
    > return 0;
    > -
    > -out:
    > + case PACKET_REJECT:
    > + reason = SKB_DROP_REASON_NOMEM;
    > + break;
    > + }
    > icmp_send(skb, ICMP_DEST_UNREACH, ICMP_PORT_UNREACH, 0);
    > drop:
    > - kfree_skb(skb);
    > + if (csum_err)
    > + reason = SKB_DROP_REASON_GRE_CSUM;
    > + kfree_skb_reason(skb, reason);
    > return 0;
    > }
    >

    \
     
     \ /
      Last update: 2022-03-16 04:18    [W:2.631 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site