Re: [PATCH net-next] net: dsa: allow port mirroring towards foreign interfaces

On Sat, Feb 06, 2021 at 03:58:57PM -0800, Jakub Kicinski wrote:

quoted

For ingress mirroring there should be nothing special about the mirror
packets, it's just more traffic in the ingress data path where the qdisc
hook already exists.

For ingress the only possible corner case seems to be if the filter has
SKIP_SW set, then HW will send to CPU but SW will ignore.

Correct, but I'm not sure if this requirement can be enforced at driver
level though.

That's assuming the frame still comes on the CPU appropriately tagged.

For ingress mirroring I think the assumption that it does is reasonable,
since the packet should be mirrored before the forwarding took place, it
can only have one DSA tag and that would be the tag where the source
port is the ingress port.
For egress mirroring, software would need to see the mirrored packet as
coming from the egress port, and this would mean that the source port in
the DSA frame header would have to be equal to the egress port.

quoted

For egress mirroring I don't think there's really any way for the mirred
action to take over the packets from what is basically the ingress qdisc
and into the egress qdisc of the DSA interface such that they will be
redirected to the selected mirror. I hadn't even thought about egress
mirroring. I suppose with more API, we could have DSA do introspection
into the frame header, see it's an egress-mirrored packet, and inject it
into the egress qdisc of the net device instead of doing netif_rx.

IMHO it's not very pretty but FWIW some "SmartNIC" drivers already do
a similar thing. But to be clear that's just an optimization, right?
The SW should still be able to re-process and come to the same
decisions as the switch, provided SKIP_SW was not set?

I guess what would need to happen is that we'd need to do something like
this, from the DSA tagging protocol files:

	if (is_egress_mirror(skb)) {
		skb_get(skb);
		skb_push(skb, ETH_ALEN);
		skb = sch_handle_egress(skb, &err, skb->dev);
		if (skb)
			consume_skb(skb);
		return NULL;
	}

basically just run whatever tc filters there might be on that packet (in
our case mirred), then discard it.

It's not an optimization thing. Egress mirrored traffic on a DSA switch
is still ingress traffic from software's perspective, so it won't match
on any mirred action on any egress qdisc. Only packets sent from the
network stack would match the mirred egress mirror rule, however there
might be lots of offloaded flows which don't.

Or I might just be misunderstanding.

quoted

The idea with 2 mirrors might work however it's not amazing and I was
thinking that if we bother to do something at all, we could as well try
to think it through and come up with something that's seamless for the
user.