| 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: <20250416123449.GQ395307@horms.kernel.org>
Date: Wed, 16 Apr 2025 13:34:49 +0100
From: Simon Horman <horms@...nel.org>
To: Dave Marquardt <davemarq@...ux.ibm.com>
Cc: netdev@...r.kernel.org, michal.swiatkowski@...ux.intel.com,
linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH net-next v2 1/2] net: ibmveth: make ibmveth use WARN_ON
instead of BUG_ON
On Mon, Apr 14, 2025 at 02:40:15PM -0500, Dave Marquardt wrote:
> Replaced BUG_ON calls with WARN_ON calls with error handling, with
> calls to a new ibmveth_reset routine, which resets the device. Removed
> conflicting and unneeded forward declaration.
To me the most important change here is adding the ibmveth_reset.
So I would report that in the subject (rather than the WARN_ON) change.
But perhaps that is just me.
>
> Signed-off-by: Dave Marquardt <davemarq@...ux.ibm.com>
> ---
> drivers/net/ethernet/ibm/ibmveth.c | 116 ++++++++++++++++++++++++-----
> drivers/net/ethernet/ibm/ibmveth.h | 65 ++++++++--------
> 2 files changed, 130 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
...
> @@ -370,20 +372,36 @@ static void ibmveth_free_buffer_pool(struct ibmveth_adapter *adapter,
> }
> }
>
> -/* remove a buffer from a pool */
> -static void ibmveth_remove_buffer_from_pool(struct ibmveth_adapter *adapter,
> - u64 correlator, bool reuse)
> +/**
> + * ibmveth_remove_buffer_from_pool - remove a buffer from a pool
> + * @adapter: adapter instance
> + * @correlator: identifies pool and index
> + * @reuse: whether to reuse buffer
The above is the correct way to document function parameters in a Kernel doc.
> + *
> + * Return:
> + * * %0 - success
> + * * %-EINVAL - correlator maps to pool or index out of range
> + * * %-EFAULT - pool and index map to null skb
> + */
> +static int ibmveth_remove_buffer_from_pool(struct ibmveth_adapter *adapter,
> + u64 correlator, bool reuse)
...
> +/**
> + * ibmveth_rxq_harvest_buffer - Harvest buffer from pool
> + *
> + * @adapter - pointer to adapter
> + * @reuse - whether to reuse buffer
But this is not correct. IOW, tooling expects
f.e. @adapter: ... rather than @adapter - ...
Flagged by W=1 builds and ./scripts/kernel-doc -none
> + *
> + * Context: called from ibmveth_poll
> + *
> + * Return:
> + * * %0 - success
> + * * other - non-zero return from ibmveth_remove_buffer_from_pool
> + */
> +static int ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter,
> + bool reuse)
...
> diff --git a/drivers/net/ethernet/ibm/ibmveth.h b/drivers/net/ethernet/ibm/ibmveth.h
> index 8468e2c59d7a..b0a2460ec9f9 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.h
> +++ b/drivers/net/ethernet/ibm/ibmveth.h
> @@ -134,38 +134,39 @@ struct ibmveth_rx_q {
> };
>
> struct ibmveth_adapter {
> - struct vio_dev *vdev;
> - struct net_device *netdev;
> - struct napi_struct napi;
> - unsigned int mcastFilterSize;
> - void * buffer_list_addr;
> - void * filter_list_addr;
> - void *tx_ltb_ptr[IBMVETH_MAX_QUEUES];
> - unsigned int tx_ltb_size;
> - dma_addr_t tx_ltb_dma[IBMVETH_MAX_QUEUES];
> - dma_addr_t buffer_list_dma;
> - dma_addr_t filter_list_dma;
> - struct ibmveth_buff_pool rx_buff_pool[IBMVETH_NUM_BUFF_POOLS];
> - struct ibmveth_rx_q rx_queue;
> - int rx_csum;
> - int large_send;
> - bool is_active_trunk;
> -
> - u64 fw_ipv6_csum_support;
> - u64 fw_ipv4_csum_support;
> - u64 fw_large_send_support;
> - /* adapter specific stats */
> - u64 replenish_task_cycles;
> - u64 replenish_no_mem;
> - u64 replenish_add_buff_failure;
> - u64 replenish_add_buff_success;
> - u64 rx_invalid_buffer;
> - u64 rx_no_buffer;
> - u64 tx_map_failed;
> - u64 tx_send_failed;
> - u64 tx_large_packets;
> - u64 rx_large_packets;
> - /* Ethtool settings */
> + struct vio_dev *vdev;
> + struct net_device *netdev;
> + struct napi_struct napi;
> + struct work_struct work;
> + unsigned int mcastFilterSize;
> + void *buffer_list_addr;
> + void *filter_list_addr;
> + void *tx_ltb_ptr[IBMVETH_MAX_QUEUES];
> + unsigned int tx_ltb_size;
> + dma_addr_t tx_ltb_dma[IBMVETH_MAX_QUEUES];
> + dma_addr_t buffer_list_dma;
> + dma_addr_t filter_list_dma;
> + struct ibmveth_buff_pool rx_buff_pool[IBMVETH_NUM_BUFF_POOLS];
> + struct ibmveth_rx_q rx_queue;
> + int rx_csum;
> + int large_send;
> + bool is_active_trunk;
> +
> + u64 fw_ipv6_csum_support;
> + u64 fw_ipv4_csum_support;
> + u64 fw_large_send_support;
> + /* adapter specific stats */
> + u64 replenish_task_cycles;
> + u64 replenish_no_mem;
> + u64 replenish_add_buff_failure;
> + u64 replenish_add_buff_success;
> + u64 rx_invalid_buffer;
> + u64 rx_no_buffer;
> + u64 tx_map_failed;
> + u64 tx_send_failed;
> + u64 tx_large_packets;
> + u64 rx_large_packets;
> + /* Ethtool settings */
> u8 duplex;
> u32 speed;
> };
If you would like to update the indentation of this structure
then please do so in a separate patch which precedes
adding/removing/chainging fields of the structure.
As it, it's very hard to see the non-formatting changes in this hunk.
--
pw-bot: changes-requested
Powered by blists - more mailing lists