Re: [PATCH net-next] net: bcmgenet: Add a check for oversized packets

[Florian Fainelli <florian.fainelli@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]

On 1/30/23 02:09, Leon Romanovsky wrote:
> On Sun, Jan 29, 2023 at 01:17:43PM -0800, Florian Fainelli wrote:
>>
>>
>> On 1/29/2023 1:42 AM, Leon Romanovsky wrote:
>>> On Thu, Jan 26, 2023 at 04:08:19PM -0800, Florian Fainelli wrote:
>>>> Occasionnaly we may get oversized packets from the hardware which
>>>> exceed the nomimal 2KiB buffer size we allocate SKBs with. Add an early
>>>> check which drops the packet to avoid invoking skb_over_panic() and move
>>>> on to processing the next packet.
>>>>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> ---
>>>>    drivers/net/ethernet/broadcom/genet/bcmgenet.c | 8 ++++++++
>>>>    1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>>>> index 21973046b12b..d937daa8ee88 100644
>>>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>>>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>>>> @@ -2316,6 +2316,14 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
>>>>    			  __func__, p_index, ring->c_index,
>>>>    			  ring->read_ptr, dma_length_status);
>>>> +		if (unlikely(len > RX_BUF_LENGTH)) {
>>>> +			netif_err(priv, rx_status, dev, "oversized packet\n");
>>>
>>> I don't think that it is wise move to print to dmesg something that can
>>> be triggered by user over network.
>>
>> A frame larger than RX_BUF_LENGTH intentionally received would be segmented
>> by the MAC, we have seen this happen however while playing with unsafe clock
>> ratios for instance or when there are insufficient credits given to the
>> Ethernet MAC to write frames into DRAM. The print is consistent with other
>> errors that are captured and is only enabled if the appropriate ethtool
>> message level bitmask is set.
> 
> I saw other prints in that function, but you add new one.
> Won't netif_err() be printed by default in almost all distro?

Do distributions alter the drive default message level:

   #define GENET_MSG_DEFAULT       (NETIF_MSG_DRV | NETIF_MSG_PROBE | \
                                   NETIF_MSG_LINK)
?
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]