netdev - Re: [PATCH/RFC 2/3] net/sched: act_tunnel

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: <20180309122248.1979e00f@redhat.com>
Date:   Fri, 9 Mar 2018 12:22:48 +0100
From:   Jiri Benc <jbenc@...hat.com>
To:     Simon Horman <simon.horman@...ronome.com>
Cc:     Jiri Pirko <jiri@...lanox.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com,
        Pieter Jansen van Vuuren 
        <pieter.jansenvanvuuren@...ronome.com>
Subject: Re: [PATCH/RFC 2/3] net/sched: act_tunnel_key: add extended ack
 support

On Tue,  6 Mar 2018 18:08:04 +0100, Simon Horman wrote:
> -	if (!tb[TCA_TUNNEL_KEY_PARMS])
> +	if (!tb[TCA_TUNNEL_KEY_PARMS]) {
> +		NL_SET_ERR_MSG(extack, "Missing tunnel key parameter");

"parameters" (it's not just one parameter)

> @@ -107,6 +109,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
>  		break;
>  	case TCA_TUNNEL_KEY_ACT_SET:
>  		if (!tb[TCA_TUNNEL_KEY_ENC_KEY_ID]) {
> +			NL_SET_ERR_MSG(extack, "Missing tunnel key enc id");

This is not much helpful to the user. What's "enc"? I guess "Missing
tunnel key id" would be enough and better.

> @@ -144,6 +147,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
>  						      0, flags,
>  						      key_id, 0);
>  		} else {
> +			NL_SET_ERR_MSG(extack, "Missing both ipv4 and ipv6 enc src and dst");

We don't need both but only one of them. And again, "enc" does not have
a clear meaning.

"Missing either IPv4 or IPv6 source and destination address"?

In addition, it makes little sense to me to add extacks to just some of
the errors in the tunnel_key_init function. Please add extacks to all
of them.

Thanks,

 Jiri