netdev - Re: [PATCH bpf-next v2 1/2] net: xdp: refactor XDP_QUERY_PROG{,

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]

Date:   Sat, 1 Jun 2019 12:57:17 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     "toke@...hat.com" <toke@...hat.com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "ast@...nel.org" <ast@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bjorn.topel@...il.com" <bjorn.topel@...il.com>,
        "magnus.karlsson@...el.com" <magnus.karlsson@...el.com>,
        "bjorn.topel@...el.com" <bjorn.topel@...el.com>,
        "brouer@...hat.com" <brouer@...hat.com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 1/2] net: xdp: refactor XDP_QUERY_PROG{,_HW}
 to netdev

On Fri, 31 May 2019 19:18:17 +0000, Saeed Mahameed wrote:
> > +	if (!bpf_op || flags & XDP_FLAGS_SKB_MODE)
> > +		mode = XDP_FLAGS_SKB_MODE;
> > +
> > +	curr_mode = dev_xdp_current_mode(dev);
> > +
> > +	if (!offload && curr_mode && (mode ^ curr_mode) &
> > +	    (XDP_FLAGS_DRV_MODE | XDP_FLAGS_SKB_MODE)) {  
> 
> if i am reading this correctly this is equivalent to :
> 
> if (!offload && (curre_mode != mode)) 
> offlad is false then curr_mode and mode must be DRV or GENERIC .. 

Naw, if curr_mode is not set, i.e. nothing installed now, we don't care
about the diff.

> better if you keep bitwise operations for actual bitmasks, mode and
> curr_mode are not bitmask, they can hold one value each .. according to
> your logic.. 

Well, they hold one bit each, whether one bit is a bitmap perhaps is
disputable? :)

I think the logic is fine.

What happened to my request to move the change in behaviour for
disabling to a separate patch, tho, Bjorn? :)