netdev - Re: [PATCH v5 6/6 resend] net: sh

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-id: <4FCC9942.5080809@renesas.com>
Date:	Mon, 04 Jun 2012 20:17:22 +0900
From:	"Shimoda, Yoshihiro" <yoshihiro.shimoda.uh@...esas.com>
To:	Jan Ceuleers <jan.ceuleers@...puter.org>
Cc:	netdev <netdev@...r.kernel.org>,
	SH-Linux <linux-sh@...r.kernel.org>
Subject: Re: [PATCH v5 6/6 resend] net: sh_eth: use NAPI

2012/06/02 19:05, Jan Ceuleers wrote:
> On 05/29/2012 10:15 AM, Shimoda, Yoshihiro wrote:
>> @@ -1087,13 +1088,17 @@ static int sh_eth_rx(struct net_device *ndev)
>>  				skb_reserve(skb, NET_IP_ALIGN);
>>  			skb_put(skb, pkt_len);
>>  			skb->protocol = eth_type_trans(skb, ndev);
>> -			netif_rx(skb);
>> -			ndev->stats.rx_packets++;
>> -			ndev->stats.rx_bytes += pkt_len;
>> +			if (netif_receive_skb(skb) == NET_RX_DROP) {
>> +				ndev->stats.rx_dropped++;
>> +			} else {
>> +				ndev->stats.rx_packets++;
>> +				ndev->stats.rx_bytes += pkt_len;
>> +			}
>>  		}
>>  		rxdesc->status |= cpu_to_edmac(mdp, RD_RACT);
>>  		entry = (++mdp->cur_rx) % mdp->num_rx_ring;
>>  		rxdesc = &mdp->rx_ring[entry];
>> +		(*work)++;
>>  	}
>>
>>  	/* Refill the Rx ring buffers. */
> 
> Please forgive a newbie's question/comment; feel free to ignore if I'm
> wasting your time. Particularly because it's about an aspect of the
> driver that you're not changing in this patch. (And yes, I know that
> you've been asked to sit on this patch series until net-next opens up
> again).
> 
> I see that most users of netif_receive_skb() ignore its return value.
> Some drivers (including this-one) do check it and use it to determine
> whether counters should be updated. But looking at netif_receive_skb()
> itself I see that there's counter infrastructure there already.
> 
> So why this in-driver set of counters, I wonder?

Thank you for the comment.
I also think that the driver should not update the rx_dropped counter
when netif_receive_skb() returns NET_RX_DROP. I will fix this.

Best regards,
Yoshihiro Shimoda
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html