netdev - RE: [iwl-next v2 03/15] ice: add basic devlink subfunctions support

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: <CO1PR11MB5089141F2033C021639AF8BFD6E22@CO1PR11MB5089.namprd11.prod.outlook.com>
Date: Mon, 13 May 2024 21:40:28 +0000
From: "Keller, Jacob E" <jacob.e.keller@...el.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>, Jiri Pirko
	<jiri@...nulli.us>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Kubiak, Michal"
	<michal.kubiak@...el.com>, "Fijalkowski, Maciej"
	<maciej.fijalkowski@...el.com>, "Samudrala, Sridhar"
	<sridhar.samudrala@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, "Drewek, Wojciech"
	<wojciech.drewek@...el.com>, "pio.raczynski@...il.com"
	<pio.raczynski@...il.com>, "jiri@...dia.com" <jiri@...dia.com>, "Polchlopek,
 Mateusz" <mateusz.polchlopek@...el.com>, "shayd@...dia.com"
	<shayd@...dia.com>
Subject: RE: [iwl-next v2 03/15] ice: add basic devlink subfunctions support



> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> Sent: Monday, May 13, 2024 4:44 AM
> To: Jiri Pirko <jiri@...nulli.us>
> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Keller, Jacob E
> <jacob.e.keller@...el.com>; Kubiak, Michal <michal.kubiak@...el.com>;
> Fijalkowski, Maciej <maciej.fijalkowski@...el.com>; Samudrala, Sridhar
> <sridhar.samudrala@...el.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@...el.com>; Drewek, Wojciech
> <wojciech.drewek@...el.com>; pio.raczynski@...il.com; jiri@...dia.com;
> Polchlopek, Mateusz <mateusz.polchlopek@...el.com>; shayd@...dia.com
> Subject: Re: [iwl-next v2 03/15] ice: add basic devlink subfunctions support
> 
> On Mon, May 13, 2024 at 01:04:23PM +0200, Jiri Pirko wrote:
> > Mon, May 13, 2024 at 10:37:23AM CEST, michal.swiatkowski@...ux.intel.com
> wrote:
> >
> > [...]
> >
> >
> >
> > >+int ice_devlink_create_sf_port(struct ice_dynamic_port *dyn_port)
> > >+{
> > >+	struct devlink_port_attrs attrs = {};
> > >+	struct devlink_port *devlink_port;
> > >+	struct devlink *devlink;
> > >+	struct ice_vsi *vsi;
> > >+	struct device *dev;
> > >+	struct ice_pf *pf;
> > >+	int err;
> > >+
> > >+	vsi = dyn_port->vsi;
> > >+	pf = dyn_port->pf;
> > >+	dev = ice_pf_to_dev(pf);
> > >+
> > >+	devlink_port = &dyn_port->devlink_port;
> > >+
> > >+	attrs.flavour = DEVLINK_PORT_FLAVOUR_PCI_SF;
> > >+	attrs.pci_sf.pf = pf->hw.bus.func;
> > >+	attrs.pci_sf.sf = dyn_port->sfnum;
> > >+
> > >+	devlink_port_attrs_set(devlink_port, &attrs);
> > >+	devlink = priv_to_devlink(pf);
> > >+
> > >+	err = devl_port_register_with_ops(devlink, devlink_port, vsi->idx,
> > >+					  &ice_devlink_port_sf_ops);
> > >+	if (err) {
> > >+		dev_err(dev, "Failed to create devlink port for Subfunction %d",
> > >+			vsi->idx);
> >
> > Either use extack or avoid this error message entirely. Could you please
> > double you don't write dmesg error messages in case you have extack
> > available in the rest of this patchset?
> >
> >
> 
> Sure, I can avoid, as this is called from port representor creeation
> function. I don't want to pass extack there (code is generic for VF and
> SF, and VF call doesn't have extack).

You can also pass an extack of NULL for flows which lack the extack, since all the extack functions are NULL-safe. Of course this does mean that you would end up with no error message logged in the VF case...

> 
> We have this pattern in few place in code (using dev_err even extack can
> be passed). Is it recommended to pass extact to all functions
> which probably want to write some message in case of error (assuming the
> call context has the extack)?
> 


Generally, yes. Extended ACK messages return and get logged on the command line of the application that issued the netlink message. This is significantly more visible than a log message from the driver.