On Tue, Oct 06, 2020 at 08:04:24AM -0700, Jakub Kicinski wrote:
> Older kernels don't support statistics, to avoid retries
> make use of netlink policy dumps to figure out which
> flags kernel actually supports.
>
> Signed-off-by: Jakub Kicinski <k...@kernel.org>
> ---
[...]
> +static int family_policy_cb(const struct nlmsghdr *nlhdr, void *data)
> +{
> + const struct nlattr *tba[NL_POLICY_TYPE_ATTR_MAX + 1] = {};
> + DECLARE_ATTR_TB_INFO(tba);
> + const struct nlattr *tb[CTRL_ATTR_MAX + 1] = {};
> + DECLARE_ATTR_TB_INFO(tb);
> + const struct nlattr *policy_attr, *attr_attr, *attr;
> + struct ethtool_op_policy_query_ctx *ctx = data;
I would rather use a different name for this variable as "ctx" is used
for struct cmd_context everywhere else.
> + unsigned int attr_idx, policy_idx;
> + int ret;
[...]
> +static int get_flags_policy(struct nl_context *nlctx, struct nl_socket *nlsk,
> + unsigned int nlcmd, unsigned int hdrattr)
> +{
> + struct nl_msg_buff *msgbuff = &nlsk->msgbuff;
> + struct ethtool_op_policy_query_ctx ctx;
Same here.
> + int ret;
> +
> + memset(&ctx, 0, sizeof(ctx));
> + ctx.nlctx = nlctx;
> + ctx.op = nlcmd;
> + ctx.op_hdr_attr = hdrattr;
> +
> + ret = __msg_init(msgbuff, GENL_ID_CTRL, CTRL_CMD_GETPOLICY,
> + NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP, 1);
> + if (ret < 0)
> + return ret;
> + ret = -EMSGSIZE;
> + if (ethnla_put_u16(msgbuff, CTRL_ATTR_FAMILY_ID, nlctx->ethnl_fam))
> + return ret;
> + if (ethnla_put_u32(msgbuff, CTRL_ATTR_OP, nlcmd))
> + return ret;
> +
> + nlsock_sendmsg(nlsk, NULL);
> + nlsock_process_reply(nlsk, family_policy_cb, &ctx);
> +
> + ret = ctx.flag_mask;
> + return ret;
> +}
The return value is assigned either a negative error code or a u32 flag
mask. If we run this code on a kernel supporting 32 flags, flag in bit
31 would collide with sign and successfully retrieved flag mask would be
interpreted as an error by caller.
Other than this, the series looks good to me.
Michal
signature.asc
Description: PGP signature
