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] [day] [month] [year] [list]

Date: Tue, 14 May 2024 10:09:36 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	jacob.e.keller@...el.com, michal.kubiak@...el.com,
	maciej.fijalkowski@...el.com, sridhar.samudrala@...el.com,
	przemyslaw.kitszel@...el.com, wojciech.drewek@...el.com,
	pio.raczynski@...il.com, jiri@...dia.com,
	mateusz.polchlopek@...el.com, shayd@...dia.com
Subject: Re: [iwl-next v2 03/15] ice: add basic devlink subfunctions support

Mon, May 13, 2024 at 01:44:14PM CEST, michal.swiatkowski@...ux.intel.com wrote:
>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).
>
>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)? 

Always.

>
>> >+		return err;
>> >+	}
>> >+
>> >+	return 0;
>> >+}
>> >+
>> 
>> [...]