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

Message-ID: <1e8bc03b-4cfb-f462-a93f-b1a12a78e86e@gmail.com>
Date:   Thu, 19 Sep 2019 15:12:43 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Jason Cobham <cobham.jason@...il.com>,
        Iwan R Timmer <irtimmer@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...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

On 9/19/19 2:30 PM, Jason Cobham wrote:
> 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?

Given that the net-next tree is closed at the moment, working offline
and posting a combined version of a patch that supports port mirroring
for cross chip configurations as well as standalone sounds good to me.

Thanks!
-- 
Florian