drivers/net/ethernet/atheros/ag71xx.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
The err variable isn't being used anywhere other than getting
initialized to 0 and then it is being checked in if condition. The
condition can never be true. Remove the err and deadcode.
Move the rx_dropped counter above when skb isn't found.
Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- Move the rx_dropped counter above when skb isn't found.
---
drivers/net/ethernet/atheros/ag71xx.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index db2a8ade62055..2effceeb191db 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -1619,7 +1619,6 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit)
unsigned int i = ring->curr & ring_mask;
struct ag71xx_desc *desc = ag71xx_ring_desc(ring, i);
int pktlen;
- int err = 0;
if (ag71xx_desc_empty(desc))
break;
@@ -1643,20 +1642,16 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit)
skb = napi_build_skb(ring->buf[i].rx.rx_buf, ag71xx_buffer_size(ag));
if (!skb) {
skb_free_frag(ring->buf[i].rx.rx_buf);
+ ndev->stats.rx_dropped++;
goto next;
}
skb_reserve(skb, offset);
skb_put(skb, pktlen);
- if (err) {
- ndev->stats.rx_dropped++;
- kfree_skb(skb);
- } else {
- skb->dev = ndev;
- skb->ip_summed = CHECKSUM_NONE;
- list_add_tail(&skb->list, &rx_list);
- }
+ skb->dev = ndev;
+ skb->ip_summed = CHECKSUM_NONE;
+ list_add_tail(&skb->list, &rx_list);
next:
ring->buf[i].rx.rx_buf = NULL;
--
2.39.2On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote:
> The err variable isn't being used anywhere other than getting
> initialized to 0 and then it is being checked in if condition. The
> condition can never be true. Remove the err and deadcode.
>
> Move the rx_dropped counter above when skb isn't found.
>
> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Thank you!
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Wed, Sep 11, 2024 at 8:20 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote:
> > The err variable isn't being used anywhere other than getting
> > initialized to 0 and then it is being checked in if condition. The
> > condition can never be true. Remove the err and deadcode.
> >
> > Move the rx_dropped counter above when skb isn't found.
> >
> > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>
> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
> Thank you!
>
> Regards,
> Oleksij
I do not see any credits given to Qianqiang Liu, who is desperate to get his
first linux patch...
https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/
On Thu, 12 Sep 2024 17:56:11 +0200 Eric Dumazet wrote:
> On Wed, Sep 11, 2024 at 8:20 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote:
> > > The err variable isn't being used anywhere other than getting
> > > initialized to 0 and then it is being checked in if condition. The
> > > condition can never be true. Remove the err and deadcode.
> > >
> > > Move the rx_dropped counter above when skb isn't found.
> > >
> > > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> >
> > Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
> I do not see any credits given to Qianqiang Liu, who is desperate to get his
> first linux patch...
>
> https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/
Right, odd, is there a reason you took over from Qianqiang Liu?
Otherwise I'd prefer if they could send the next version.
Last thing we need is arguments about ownership of trivial
patches.
This v2 has an unnecessary Fixes tag, this is not a fix.
On Thu, 12 Sep 2024 17:42:22 -0700 Jakub Kicinski wrote: > On Thu, 12 Sep 2024 17:56:11 +0200 Eric Dumazet wrote: > > I do not see any credits given to Qianqiang Liu, who is desperate to get his > > first linux patch... > > > > https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/ > > Right, odd, is there a reason you took over from Qianqiang Liu? > Otherwise I'd prefer if they could send the next version. > Last thing we need is arguments about ownership of trivial > patches. > > This v2 has an unnecessary Fixes tag, this is not a fix. Oh, I guess it may be a v2 of your own change: https://lore.kernel.org/all/20240911105924.4028423-1-usama.anjum@collabora.com/ Are you both using the same tool? 5 year old code and suddenly we get the same fix two times.
Hi Eric, > I do not see any credits given to Qianqiang Liu, who is desperate to get his > first linux patch... > > https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/ Yes, you are right! I'm a kernel newbie. But Linux is a FOSS software, and anyone can contribute, right? Actually, I have two patches that were merged into the linux-next branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=93497752dfed196b41d2804503e80b9a04318adb https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=cd0920ebab6bce93ac5054d621c0633f6a4d640b -- Best, Qianqiang Liu
On 11/09/2024 14:58, Muhammad Usama Anjum wrote:
> The err variable isn't being used anywhere other than getting
> initialized to 0 and then it is being checked in if condition. The
> condition can never be true. Remove the err and deadcode.
>
> Move the rx_dropped counter above when skb isn't found.
>
> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - Move the rx_dropped counter above when skb isn't found.
> ---
> drivers/net/ethernet/atheros/ag71xx.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
> index db2a8ade62055..2effceeb191db 100644
> --- a/drivers/net/ethernet/atheros/ag71xx.c
> +++ b/drivers/net/ethernet/atheros/ag71xx.c
> @@ -1619,7 +1619,6 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit)
> unsigned int i = ring->curr & ring_mask;
> struct ag71xx_desc *desc = ag71xx_ring_desc(ring, i);
> int pktlen;
> - int err = 0;
>
> if (ag71xx_desc_empty(desc))
> break;
> @@ -1643,20 +1642,16 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit)
> skb = napi_build_skb(ring->buf[i].rx.rx_buf, ag71xx_buffer_size(ag));
> if (!skb) {
> skb_free_frag(ring->buf[i].rx.rx_buf);
> + ndev->stats.rx_dropped++;
> goto next;
> }
>
> skb_reserve(skb, offset);
> skb_put(skb, pktlen);
>
> - if (err) {
> - ndev->stats.rx_dropped++;
> - kfree_skb(skb);
> - } else {
> - skb->dev = ndev;
> - skb->ip_summed = CHECKSUM_NONE;
> - list_add_tail(&skb->list, &rx_list);
> - }
> + skb->dev = ndev;
> + skb->ip_summed = CHECKSUM_NONE;
> + list_add_tail(&skb->list, &rx_list);
>
> next:
> ring->buf[i].rx.rx_buf = NULL;
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote:
> The err variable isn't being used anywhere other than getting
> initialized to 0 and then it is being checked in if condition. The
> condition can never be true. Remove the err and deadcode.
>
> Move the rx_dropped counter above when skb isn't found.
>
> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
© 2016 - 2024 Red Hat, Inc.