netdev - RE: [PATCH net-next v2 11/13] ice: enable FDIR filters from raw binary patterns for VFs

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: <BL3PR11MB64822BAEBF130476C4F6C9E3E7552@BL3PR11MB6482.namprd11.prod.outlook.com>
Date: Thu, 31 Oct 2024 07:37:39 +0000
From: "Guo, Junfeng" <junfeng.guo@...el.com>
To: "Keller, Jacob E" <jacob.e.keller@...el.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "Zaki, Ahmed" <ahmed.zaki@...el.com>, "Chittim,
 Madhu" <madhu.chittim@...el.com>
CC: "horms@...nel.org" <horms@...nel.org>, "hkelam@...vell.com"
	<hkelam@...vell.com>, Marcin Szycik <marcin.szycik@...ux.intel.com>,
	"Romanowski, Rafal" <rafal.romanowski@...el.com>
Subject: RE: [PATCH net-next v2 11/13] ice: enable FDIR filters from raw
 binary patterns for VFs

Hi Jake, 

Thanks for pointing this out with your carefully review!

And yes, you are correct. 
The implementation of fdir_prof_info as a table may waste a lot of memory.
It would be better to use a linked list instead to save memory.
And the logic of store/find/delete profile info should be modified accordingly.

Unfortunately, I'm not able to working on the improvement now. : (
Hi @Chittim, Madhu and @Zaki, Ahmed, could you help consider about how to improve the code?
Thanks a lot!

Regards,
Junfeng

> -----Original Message-----
> From: Keller, Jacob E <jacob.e.keller@...el.com>
> Sent: Thursday, October 31, 2024 08:12
> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> davem@...emloft.net; kuba@...nel.org; pabeni@...hat.com;
> edumazet@...gle.com; netdev@...r.kernel.org
> Cc: Guo, Junfeng <junfeng.guo@...el.com>; Zaki, Ahmed
> <ahmed.zaki@...el.com>; Chittim, Madhu <madhu.chittim@...el.com>;
> horms@...nel.org; hkelam@...vell.com; Marcin Szycik
> <marcin.szycik@...ux.intel.com>; Romanowski, Rafal
> <rafal.romanowski@...el.com>
> Subject: Re: [PATCH net-next v2 11/13] ice: enable FDIR filters from raw
> binary patterns for VFs
> 
> Hi Junfeng Guo,
> 
> I realize this is a bit late since this code already merged, but I was
> looking at the ice_vf structure and found it was extremely large now,
> and traced it back to this change:
> 
> On 8/13/2024 3:22 PM, Tony Nguyen wrote:
> > +/* Structure to store fdir fv entry */
> > +struct ice_fdir_prof_info {
> > +	struct ice_parser_profile prof;
> > +	u64 fdir_active_cnt;
> > +};
> > +
> >  /* VF operations */
> >  struct ice_vf_ops {
> >  	enum ice_disq_rst_src reset_type;
> > @@ -91,6 +98,7 @@ struct ice_vf {
> >  	u16 lan_vsi_idx;		/* index into PF struct */
> >  	u16 ctrl_vsi_idx;
> >  	struct ice_vf_fdir fdir;
> > +	struct ice_fdir_prof_info fdir_prof_info[ICE_MAX_PTGS];
> >  	/* first vector index of this VF in the PF space */
> >  	int first_vector_idx;
> >  	struct ice_sw *vf_sw_id;	/* switch ID the VF VSIs connect to */
> This adds 135,168 bytes to every single VF structure.
> 
> Is there no reason we can't convert this into some sort of linked list
> so we don't have to store every possible ice_fdir_prof_info even if its
> not used??
> 
> It seems like even if we need to store all 256 entries, we could
> allocate them as a list, xarray, or something which would prevent
> needing to allocate them in an entire chunk like we do with a VF
> structure. We could also theoretically allocate it on demand only when
> these raw patterns are used?
> 
> Forcing the driver to consume 132KB of memory for every VF seems rather
> overkill. If we create 128 VFs, this consumes 16.5 MB of memory.
> 
> Thanks,
> Jake