netdev - Re: [PATCH net-next] net: dsa: mv88e6xxx: Add support for port mirroring

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-next>] [day] [month] [year] [list]

Message-ID: <CAKu_b=+0=KXnT-b8L2qkUxT2jSMAJaiMBNAeSjJ3hPqZgx4PGw@mail.gmail.com>
Date:   Thu, 19 Sep 2019 14:30:06 -0700
From:   Jason Cobham <cobham.jason@...il.com>
To:     Iwan R Timmer <irtimmer@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: Add support for port mirroring

Hi Iwan,

>Hi Andrew,
>
>I only own a simple 5 ports switch (88E6176) which has no problem of mirroring the other ports to a single port. Except for a bandwith shortage ofcourse. While I thought I checked adding and removing ports, I seemed to forgot to check removing ingress traffic as it will now >disable mirroring egress traffic. Searching for how I can distinct ingress from egress mirroring in port_mirror_del, I saw there is a variable in the mirror struct called ingress. Which seems strange, because why is it a seperate argument to the port_mirror_add function?
>
>Origally I planned to be able to set the egress and ingress mirror seperatly. But in my laziness when I saw there already was a function to configure the destination port this functionality was lost.
>
>Because the other drivers which implemented the port_mirror_add (b53 and
>ksz9477) also lacks additional checks to prevent new mirror filters from breaking previous ones I assumed they were not necessary.
>
>At least I will soon sent a new version with at least the issue of removing mirror ingress traffic fixed and the ability to define a seperate ingress and egress port.
>
>Regards,
>Iwan

I have a similar patch set for port mirror from a few years ago. I'd
also like to see this functionality in mainline. One issue I ran into
is when doing port mirror in a cross-chip dsa configuration. If the
ingress and egress ports are on different chips, the ingress chip
needs to set the egress to the cross-chip dsa port and the cross-chip
egress port needs to be set appropriately. I also had the
functionality to mirror egress from a port to a destination port.

Is it appropriate to send my patch to the mailing list for review or
should we work on this off-line?

Thanks,
Jason