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: <20230821073000.GC2711035@kernel.org>
Date: Mon, 21 Aug 2023 09:30:00 +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 09:20:37AM +0200, Simon Horman wrote:
> 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.

Sorry, somehow I wasn't looking at the entire series.
I now see that ice_parser.c is compiled as of patch 12/15 of this series.

> 
> >  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.

And I see this field is added in patch 10/15, rather than the last patch
(15/15) as I previously stated.

> 
> > +
> > +	*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);
> > +}
>