Subject: Re: [PATCH net 1/1 v4] rtnetlink: require unique netns identifier

From: Jiri Benc <[email-masked]>
Date: Wed, 7 Feb 2018 14:53:26 +0100

On Wed, 7 Feb 2018 14:36:21 +0100, Christian Brauner wrote:
> On Wed, Feb 07, 2018 at 04:20:01PM +0300, Kirill Tkhai wrote:
> > Can't we write these 3 above branches more compact? Something like this:
> > 
> > 	if (!!tb[IFLA_NET_NS_FD] + !!tb[IFLA_IF_NETNSID] + !!tb[IFLA_NET_NS_PID] <= 1)
> > 		return 0;  
> 
> I always prefer for conditions to be separate and readable even if it
> means additional code. But if others feel that there's value in avoiding
> two additional conditions I'm happy to adapt the patch.

FWIW, I don't like the n x n conditions much. But Kirill's proposal
seems not to be much better. I was thinking about:

	int cnt = 0;
	if (tb[IFLA_NET_NS_FD])
		cnt++;
	if (tb[IFLA_NET_NS_PID])
		cnt++;
	if (tb[IFLA_NET_NETNSID])
		cnt++;
	if (cnt > 1) {
		...errorr...
	}

but that's not better, either. As we're unlikely to add a fourth value,
I guess I'm okay with the current approach in the patch.

> Before I added support for netns ids for additional requests Jiri made
> it so that all request that specified properties that they did not
> support returned ENOTSUPP instead of EINVAL. This just keeps things
> consistent. Users would now suddenly receive EINVAL. That's potentially
> confusing.

Yes, please, keep -EOPNOTSUPP.

> As for the case of passing multiple netns identifying properties into
> the same request EINVAL seems the perfect candidate and the error
> message seems instructive to userspace programs.

Agreed.

Acked-by: Jiri Benc <[email-masked]>

Thanks,

 Jiri