netdev - Re: [PATCH iwl-next v5 01/15] ice: add parser create and destroy skeleton

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: <20230821072037.GB2711035@kernel.org>
Date: Mon, 21 Aug 2023 09:20:37 +0200
From: Simon Horman <horms@...nel.org>
To: Junfeng Guo <junfeng.guo@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	anthony.l.nguyen@...el.com, jesse.brandeburg@...el.com,
	qi.z.zhang@...el.com, ivecera@...hat.com,
	sridhar.samudrala@...el.com
Subject: Re: [PATCH iwl-next v5 01/15] ice: add parser create and destroy
 skeleton

On Mon, Aug 21, 2023 at 10:38:19AM +0800, Junfeng Guo wrote:
> Add new parser module which can parse a packet in binary
> and generate information like ptype, protocol/offset pairs
> and flags which can be used to feed the FXP profile creation
> directly.
> 
> The patch added skeleton of the create and destroy APIs:
> ice_parser_create
> ice_parser_destroy
> 
> Signed-off-by: Junfeng Guo <junfeng.guo@...el.com>

Hi Junfeng Guo,

some minor feedback from my side.

> ---
>  drivers/net/ethernet/intel/ice/ice_common.h |  4 +++
>  drivers/net/ethernet/intel/ice/ice_ddp.c    | 10 +++---
>  drivers/net/ethernet/intel/ice/ice_ddp.h    | 13 ++++++++
>  drivers/net/ethernet/intel/ice/ice_parser.c | 34 +++++++++++++++++++++

Perhaps I am missing something, but it seems that although
ice_parser.c is added by this patch-set, it is not added to
the build by this patch-set. This seems a little odd to me.

>  drivers/net/ethernet/intel/ice/ice_parser.h | 13 ++++++++
>  5 files changed, 69 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/net/ethernet/intel/ice/ice_parser.c
>  create mode 100644 drivers/net/ethernet/intel/ice/ice_parser.h

...

> diff --git a/drivers/net/ethernet/intel/ice/ice_parser.c b/drivers/net/ethernet/intel/ice/ice_parser.c
> new file mode 100644
> index 000000000000..42602cac7e45
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/ice_parser.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2023 Intel Corporation */
> +
> +#include "ice_common.h"
> +
> +/**
> + * ice_parser_create - create a parser instance
> + * @hw: pointer to the hardware structure
> + * @psr: output parameter for a new parser instance be created
> + */
> +int ice_parser_create(struct ice_hw *hw, struct ice_parser **psr)
> +{
> +	struct ice_parser *p;
> +
> +	p = devm_kzalloc(ice_hw_to_dev(hw), sizeof(struct ice_parser),
> +			 GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	p->hw = hw;
> +	p->rt.psr = p;

It is, perhaps academic if this file isn't compiled, but the rt field of
struct ice_parser doesn't exist at this point of the patch-set: it is added
by the last patch of the patch-set.

> +
> +	*psr = p;
> +	return 0;
> +}
> +
> +/**
> + * ice_parser_destroy - destroy a parser instance
> + * @psr: pointer to a parser instance
> + */
> +void ice_parser_destroy(struct ice_parser *psr)
> +{
> +	devm_kfree(ice_hw_to_dev(psr->hw), psr);
> +}