netdev - Re: [PATCH net-next] net/hanic: Add the hanic network interface for high availability links

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y3zXT3SmTEwjQ7y6@nanopsycho>
Date:   Tue, 22 Nov 2022 15:06:07 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Steve Williams <steve.williams@...cruise.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net/hanic: Add the hanic network interface for
 high availability links

Tue, Nov 22, 2022 at 02:55:29PM CET, olteanv@...il.com wrote:
>Hi Jiri,
>
>On Tue, Nov 22, 2022 at 01:49:38PM +0100, Jiri Pirko wrote:
>> Sat, Nov 19, 2022 at 12:26:39AM CET, steve.williams@...cruise.com wrote:
>> >This is a virtual device that implements support for 802.1cb R-TAGS
>> >and duplication and deduplication. The hanic nic itself is not a device,
>> >but enlists ethernet nics to act as parties in a high-availability
>> >link. Outbound packets are duplicated and tagged with R-TAGs, then
>> >set out the enlisted links. Inbound packets with R-TAGs have their
>> >R-TAGs removed, and duplicates are dropped to complete the link. The
>> >algorithm handles links being completely disconnected, sporadic packet
>> >loss, and out-of-order arrivals.
>> >
>> >To the extent possible, the link is self-configuring: It detects and
>> >brings up streams as R-TAG'ed packets are detected, and creates streams
>> >for outbound packets unless explicitly filtered to skip tagging.
>> >---
>> > Documentation/networking/hanic.rst |  351 ++++++++++
>> > Documentation/networking/index.rst |    1 +
>> > MAINTAINERS                        |    6 +
>> > drivers/net/Kconfig                |   17 +
>> > drivers/net/Makefile               |    1 +
>> > drivers/net/hanic/Makefile         |   15 +
>> > drivers/net/hanic/hanic_dev.c      | 1006 ++++++++++++++++++++++++++++
>> > drivers/net/hanic/hanic_filter.c   |  172 +++++
>> > drivers/net/hanic/hanic_main.c     |  109 +++
>> > drivers/net/hanic/hanic_netns.c    |   58 ++
>> > drivers/net/hanic/hanic_priv.h     |  408 +++++++++++
>> > drivers/net/hanic/hanic_protocol.c |  350 ++++++++++
>> > drivers/net/hanic/hanic_streams.c  |  161 +++++
>> > drivers/net/hanic/hanic_sysfs.c    |  672 +++++++++++++++++++
>> > 14 files changed, 3327 insertions(+)
>> 
>> Leaving aside issues I spotted looking at random parts of the code (like
>> checking if kernel version is >5 :O), why this has to be another
>> master-slave device? From the first look, I think this could be
>> implemented as a bond/team mode. You would save a lot of plumbing code
>> and ease up the maintainance burden. Did you consider that option?
>> Any particular arguments against that approach?
>
>Neither bond nor team have forwarding between ports built in, right?
>Forwarding is pretty fundamental to 802.1CB (at least to the use cases
>I know of).

I don't see any forwarding in this patch. How is it supposed to be
working, I wonder...