Re: [PATCH v2 01/13] net/mlx5: add representor recognition on kernels 5.x

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, March 25, 2019 20:08
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Shahaf Shuler <shahafs@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH v2 01/13] net/mlx5: add representor
> recognition on kernels 5.x
> 
> On Mon, 25 Mar 2019 17:03:22 +0000
> Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:
> 
> > +	if (switch_id_set) {
> > +		if (info.port_name_new) {
> > +			/* New representors naming schema. */
> > +			if (port_name_set) {
> > +				info.master = (info.port_name == -1);
> > +				info.representor = (info.port_name != -1);
> > +			}
> > +		} else {
> > +			/* Legacy representors naming schema. */
> > +			info.master = (!port_name_set || num_vf_set);
> > +			info.representor = port_name_set && !num_vf_set;
> > +		}
> > +	}
> > +	assert(!(data.master && data.representor));
> >  	memcpy(arg, &info, sizeof(info));
> >  	return 0;
> 
> Since assert() is often removed in non-debug envirionments,
Assert should be removed (compiled out) in non-debug environments. If not (suddenly) - it must be resolved.

> why not add a log message and return an error instead?
Because there is no code producing assert wrong condition. No valid combination of entry values to produce master&&representor. It is intended by design. We do not expect master&&representor condition at all. It should never happen. Otherwise, we should notify the developer (because design is corrupted somewhere - and definitely it is not a runtime/user problem). So, assert seems to be the most relevant entity here.

>>instead?
We could add error/log in parallel, not instead of.

With best regards,
Slava