netdev - Re: [PATCH net-next] drivers/net/wan/hdlc_fr: Improve fr

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: <20201018150517.2f3dfb5c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Sun, 18 Oct 2020 15:05:17 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Xie He <xie.he.0141@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Krzysztof Halasa <khc@...waw.pl>
Subject: Re: [PATCH net-next] drivers/net/wan/hdlc_fr: Improve fr_rx and add
 support for any Ethertype

On Fri, 16 Oct 2020 22:19:51 -0700 Xie He wrote:
> 1. Change the fr_rx function to make this driver support any Ethertype
> when receiving. (This driver is already able to handle any Ethertype
> when sending.)
> 
> Originally in the fr_rx function, the code that parses the long (10-byte)
> header only recognizes a few Ethertype values and drops frames with other
> Ethertype values. This patch replaces this code to make fr_rx support
> any Ethertype. This patch also creates a new function fr_snap_parse as
> part of the new code.
> 
> 2. Change the use of the "dev" variable in fr_rx. Originally we do
> "dev = something", and then at the end do "if (dev) skb->dev = dev".
> Now we do "if (something) skb->dev = something", then at the end do
> "dev = skb->dev".
> 
> This is to make the logic of our code consistent with eth_type_trans
> (which we call). The eth_type_trans function expects a non-NULL pointer
> as a parameter and assigns it directly to skb->dev.
> 
> 3. Change the initial skb->len check in fr_fx from "<= 4" to "< 4".
> At first we only need to ensure a 4-byte header is present. We indeed
> normally need the 5th byte, too, but it'd be more logical to check its
> existence when we actually need it.
> 
> Also add an fh->ea2 check to the initial checks in fr_fx. fh->ea2 == 1
> means the second address byte is the final address byte. We only support
> the case where the address length is 2 bytes.
> 
> 4. Use "goto rx_drop" whenever we need to drop a valid frame.

Whenever you make a list like that it's a strong indication that 
each of these should be a separate commit. That makes things easier 
to review.


We have already sent a pull request for 5.10 and therefore net-next 
is closed for new drivers, features, and code refactoring.

Please repost when net-next reopens after 5.10-rc1 is cut.

(http://vger.kernel.org/~davem/net-next.html will not be up to date 
 this time around, sorry about that).

RFC patches sent for review only are obviously welcome at any time.