netdev - Re: [bpf-next V1-RFC PATCH 01/14] xdp: base API for new XDP rx-queue info concept

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: <3b781865-be2a-740b-8403-fe47fea929bc@gmail.com>
Date:   Wed, 13 Dec 2017 19:34:40 -0700
From:   David Ahern <dsahern@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>,
        Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     netdev@...r.kernel.org, gospo@...adcom.com, bjorn.topel@...el.com,
        michael.chan@...adcom.com
Subject: Re: [bpf-next V1-RFC PATCH 01/14] xdp: base API for new XDP rx-queue
 info concept

On 12/13/17 4:19 AM, Jesper Dangaard Brouer wrote:
> +
> +void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq)
> +{
> +	xdp_rxq->reg_state = REG_STATE_UNREGISTRED;
> +}
> +EXPORT_SYMBOL_GPL(xdp_rxq_info_unreg);
> +
> +void xdp_rxq_info_init(struct xdp_rxq_info *xdp_rxq)
> +{
> +	if (xdp_rxq->reg_state == REG_STATE_REGISTRED) {
> +		WARN(1, "Missing unregister, handled but fix driver\n");
> +		xdp_rxq_info_unreg(xdp_rxq);
> +	}
> +	memset(xdp_rxq, 0, sizeof(*xdp_rxq));
> +	xdp_rxq->queue_index = U32_MAX;
> +	xdp_rxq->reg_state = REG_STATE_NEW;
> +}
> +EXPORT_SYMBOL_GPL(xdp_rxq_info_init);
> +
> +void xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq)
> +{
> +	WARN(!xdp_rxq->dev, "Missing net_device from driver");
> +	WARN(xdp_rxq->queue_index == U32_MAX, "Miss queue_index from driver");
> +	WARN(!(xdp_rxq->reg_state == REG_STATE_NEW),"API violation, miss init");
> +	xdp_rxq->reg_state = REG_STATE_REGISTRED;
> +}
> +EXPORT_SYMBOL_GPL(xdp_rxq_info_reg);
> 

Rather than WARN()'s why not make the _reg and _init functions return an
int that indicates an error? For example you don't want to continue if
the dev is expected but missing.