lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+FuTSfKzKZ02st-enPfsgaQwTunPrmyK2x2jobZrWGb16KN0w@mail.gmail.com> Date: Sat, 31 Oct 2020 10:32:53 -0400 From: Willem de Bruijn <willemdebruijn.kernel@...il.com> To: Xie He <xie.he.0141@...il.com> Cc: Jakub Kicinski <kuba@...nel.org>, "David S. Miller" <davem@...emloft.net>, Network Development <netdev@...r.kernel.org>, linux-kernel <linux-kernel@...r.kernel.org>, Willem de Bruijn <willemdebruijn.kernel@...il.com>, Krzysztof Halasa <khc@...waw.pl> Subject: Re: [PATCH net-next v6 1/5] net: hdlc_fr: Simpify fr_rx by using "goto rx_drop" to drop frames On Fri, Oct 30, 2020 at 8:50 PM Xie He <xie.he.0141@...il.com> wrote: > > When the fr_rx function drops a received frame (because the protocol type > is not supported, or because the PVC virtual device that corresponds to > the DLCI number and the protocol type doesn't exist), the function frees > the skb and returns. > > The code for freeing the skb and returning is repeated several times, this > patch uses "goto rx_drop" to replace them so that the code looks cleaner. > > Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com> > Cc: Krzysztof Halasa <khc@...waw.pl> > Signed-off-by: Xie He <xie.he.0141@...il.com> > --- > drivers/net/wan/hdlc_fr.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c > index 409e5a7ad8e2..4db0e01b96a9 100644 > --- a/drivers/net/wan/hdlc_fr.c > +++ b/drivers/net/wan/hdlc_fr.c > @@ -904,8 +904,7 @@ static int fr_rx(struct sk_buff *skb) > netdev_info(frad, "No PVC for received frame's DLCI %d\n", > dlci); > #endif > - dev_kfree_skb_any(skb); > - return NET_RX_DROP; > + goto rx_drop; > } > > if (pvc->state.fecn != fh->fecn) { > @@ -963,14 +962,12 @@ static int fr_rx(struct sk_buff *skb) > default: > netdev_info(frad, "Unsupported protocol, OUI=%x PID=%x\n", > oui, pid); > - dev_kfree_skb_any(skb); > - return NET_RX_DROP; > + goto rx_drop; > } > } else { > netdev_info(frad, "Unsupported protocol, NLPID=%x length=%i\n", > data[3], skb->len); > - dev_kfree_skb_any(skb); > - return NET_RX_DROP; > + goto rx_drop; > } > > if (dev) { > @@ -982,12 +979,12 @@ static int fr_rx(struct sk_buff *skb) > netif_rx(skb); > return NET_RX_SUCCESS; > } else { > - dev_kfree_skb_any(skb); > - return NET_RX_DROP; > + goto rx_drop; > } > > - rx_error: > +rx_error: > frad->stats.rx_errors++; /* Mark error */ > +rx_drop: > dev_kfree_skb_any(skb); > return NET_RX_DROP; I meant that I don't think errors should be double counted in rx_error and rx_drop. It is fine to count drops as either. Especially without that, I'm not sure this and the follow-on patch add much value. Minor code cleanups complicate backports of fixes.
Powered by blists - more mailing lists