Hi Ben, Thanx for the review. Please see inline.
On 08/05/18, 9:12 PM, "Ben Pfaff" <[email protected]> wrote: On Tue, May 08, 2018 at 08:13:10AM +0000, Manohar Krishnappa Chidambaraswamy wrote: > Problem: > ======== > Received LACP/CFM/BFD/STP/LLDP slow protocols' packets are not captured in > ovs-tcpdump. > > Fix: > ==== > Add mirror support for slow protocols. > > Signed-off-by: Manohar K C > <[email protected]> > CC: Jan Scheurich <[email protected]> Thanks for working on this. I see that there are two calls to process_special(): one in xlate_actions(), the other in patch_port_output(). Currently, it looks like patch_port_output() doesn't ever call mirror_ingress_packet(). This might be a bug of its own, but adding a call to mirror_ingress_packet() only for the case when special processing is needed is an oddity. So, the following seems like a more consistent choice: diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 5641724a7fa2..fe5fec77e5b4 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -7211,6 +7211,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) * * We do not perform special processing on thawed packets, since that * was done before they were frozen and should not be redone. */ + mirror_ingress_packet(&ctx); } else if (in_port && in_port->xbundle && xbundle_mirror_out(xbridge, in_port->xbundle)) { xlate_report_error(&ctx, "dropping packet received on port " Separately, it might be a good idea to add proper ingress mirroring to patch_port_output(). What do you think? [manu] I agree. I will send v2 with this change to cover xlate_actions(). Also I think it would only be STP(among the ones handled in process_special()) traffic that takes patch_port_output() path. Thanx Manu Thanks, Ben. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
