netdev - Re: [RFC PATCH ethtool] ethtool: Improve compatibility between netlink and ioctl interfaces

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: <20201103142430.GA951743@shredder>
Date:   Tue, 3 Nov 2020 16:24:30 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     netdev@...r.kernel.org, kuba@...nel.org, mlxsw@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [RFC PATCH ethtool] ethtool: Improve compatibility between
 netlink and ioctl interfaces

On Mon, Nov 02, 2020 at 11:58:03PM +0100, Michal Kubecek wrote:
> On Mon, Nov 02, 2020 at 08:40:36PM +0200, Ido Schimmel wrote:
> > +static int linkmodes_reply_adver_all_cb(const struct nlmsghdr *nlhdr,
> 
>                               ^^^^^ advert?
> 
> > +					void *data)
> > +{
> > +	const struct nlattr *bitset_tb[ETHTOOL_A_BITSET_MAX + 1] = {};
> > +	const struct nlattr *tb[ETHTOOL_A_LINKMODES_MAX + 1] = {};
> > +	DECLARE_ATTR_TB_INFO(bitset_tb);
> > +	struct nl_context *nlctx = data;
> > +	struct nl_msg_buff *msgbuff;
> > +	DECLARE_ATTR_TB_INFO(tb);
> > +	struct nl_socket *nlsk;
> > +	struct nlattr *nest;
> > +	int ret;
> > +
> > +	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (!tb[ETHTOOL_A_LINKMODES_OURS])
> > +		return -EINVAL;
> > +
> > +	ret = mnl_attr_parse_nested(tb[ETHTOOL_A_LINKMODES_OURS], attr_cb,
> > +				    &bitset_tb_info);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (!bitset_tb[ETHTOOL_A_BITSET_SIZE] ||
> > +	    !bitset_tb[ETHTOOL_A_BITSET_VALUE] ||
> > +	    !bitset_tb[ETHTOOL_A_BITSET_MASK])
> > +		return -EINVAL;
> > +
> > +	ret = netlink_init_ethnl2_socket(nlctx);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	nlsk = nlctx->ethnl2_socket;
> > +	msgbuff = &nlsk->msgbuff;
> > +
> > +	ret = msg_init(nlctx, msgbuff, ETHTOOL_MSG_LINKMODES_SET,
> > +		       NLM_F_REQUEST | NLM_F_ACK);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (ethnla_fill_header(msgbuff, ETHTOOL_A_LINKMODES_HEADER,
> > +			       nlctx->devname, 0))
> > +		return -EMSGSIZE;
> > +
> > +	if (ethnla_put_u8(msgbuff, ETHTOOL_A_LINKMODES_AUTONEG, AUTONEG_ENABLE))
> > +		return -EMSGSIZE;
> > +
> > +	/* Use the size and mask from the reply and set the value to the mask,
> > +	 * so that all supported link modes will be advertised.
> > +	 */
> > +	ret = -EMSGSIZE;
> > +	nest = ethnla_nest_start(msgbuff, ETHTOOL_A_LINKMODES_OURS);
> > +	if (!nest)
> > +		return -EMSGSIZE;
> > +
> > +	if (ethnla_put_u32(msgbuff, ETHTOOL_A_BITSET_SIZE,
> > +			   mnl_attr_get_u32(bitset_tb[ETHTOOL_A_BITSET_SIZE])))
> > +		goto err;
> > +
> > +	if (ethnla_put(msgbuff, ETHTOOL_A_BITSET_VALUE,
> > +		       mnl_attr_get_payload_len(bitset_tb[ETHTOOL_A_BITSET_MASK]),
> > +		       mnl_attr_get_payload(bitset_tb[ETHTOOL_A_BITSET_MASK])))
> > +		goto err;
> > +
> > +	if (ethnla_put(msgbuff, ETHTOOL_A_BITSET_MASK,
> > +		       mnl_attr_get_payload_len(bitset_tb[ETHTOOL_A_BITSET_MASK]),
> > +		       mnl_attr_get_payload(bitset_tb[ETHTOOL_A_BITSET_MASK])))
> > +		goto err;
> > +
> > +	ethnla_nest_end(msgbuff, nest);
> 
> To fully replicate ioctl code behaviour, we should only set the bits
> corresponding to "real" link modes, not "special" ones (e.g.
> ETHTOOL_LINK_MODE_TP_BIT).

Michal,

I have the changes you requested here:
https://github.com/idosch/ethtool/commit/b34d15839f2662808c566c04eda726113e20ee59

Do you want to integrate it with your nl_parse() rework or should I?

Thanks