netdev - Re: [PATCH net-next v3 3/6] net: tn40xx: add basic Tx handling

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <c2b5177c-3782-44fb-b7b0-d3ca610af1b8@lunn.ch>
Date: Tue, 30 Apr 2024 22:40:47 +0200
From: Andrew Lunn <andrew@...n.ch>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: netdev@...r.kernel.org, jiri@...nulli.us, horms@...nel.org
Subject: Re: [PATCH net-next v3 3/6] net: tn40xx: add basic Tx handling

> --- a/drivers/net/ethernet/tehuti/tn40.c
> +++ b/drivers/net/ethernet/tehuti/tn40.c
> @@ -3,10 +3,1177 @@
>  
> +static inline void tn40_do_tx_db_ptr_next(struct tn40_txdb *db,
> +					  struct tn40_tx_map **pptr)

inline functions are not liked in .c files. Leave it to the compiler
to decide.

> +{
> +	++*pptr;
> +	if (unlikely(*pptr == db->end))
> +		*pptr = db->start;
> +}
> +
> +static inline void tn40_tx_db_inc_rptr(struct tn40_txdb *db)
> +{
> +	tn40_do_tx_db_ptr_next(db, &db->rptr);
> +}
> +
> +static inline void tn40_tx_db_inc_wptr(struct tn40_txdb *db)
> +{
> +	tn40_do_tx_db_ptr_next(db, &db->wptr);
> +}

Functions like this are likely to be inlined even without the keyword.
Please look through all the code and remove the inline keyword from .c
files. They are O.K. in headers, so long as they are static inline.

> +#include <linux/delay.h>
> +#include <linux/etherdevice.h>
> +#include <linux/firmware.h>
> +#include <linux/if_ether.h>
> +#include <linux/if_vlan.h>
> +#include <linux/in.h>
> +#include <linux/interrupt.h>
> +#include <linux/ip.h>
>  #include <linux/module.h>
> +#include <linux/netdevice.h>
>  #include <linux/pci.h>
> +#include <linux/phy.h>
> +#include <linux/tcp.h>
> +#include <linux/udp.h>

More headers which should be in the .c file.

>  
>  #include "tn40_regs.h"

> +/* netdev tx queue len for Luxor. The default value is 1000.
> + * ifconfig eth1 txqueuelen 3000 - to change it at runtime.
> + */
> +#define TN40_NDEV_TXQ_LEN 3000

This comment does not seem to match the #define?

     Andrew