On 7/17/23 20:47, Mike Pattrick wrote: > Currently OVS keeps track of which mirrors that each packet has been > sent to for the purpose of deduplication. However, this doesn't consider > that openflow rules can make significant changes to packets after > ingress. > > For example, OVN can create OpenFlow rules that turn an echo request > into an echo response by flipping source/destination addresses and > setting the ICMP type to Reply. When a mirror is configured, only the > request gets mirrored even though a response is recieved. > > This can cause a false impression of the actual traffic on wire if > someone inspects the mirror and doesn't see an echo reply even though > one has been sent. > > This patch resets the mirrors every time a packet is modified, so > mirrors will recieve every copy of a packet that is sent for output.
Nit: s/recieve/receive/ in two places above. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2155579 > Signed-off-by: Mike Pattrick <[email protected]> > --- > ofproto/ofproto-dpif-xlate.c | 97 ++++++++++++++++++++++++++++++++++++ > tests/ofproto-dpif.at | 6 +-- > 2 files changed, 100 insertions(+), 3 deletions(-) > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 4928ea99c..5563ac292 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -7046,6 +7046,101 @@ xlate_ofpact_unroll_xlate(struct xlate_ctx *ctx, > "cookie=%#"PRIx64, a->rule_table_id, a->rule_cookie); > } > > +/* Reset the mirror context if we modify the packet and would like to mirror > + * the new copy. */ > +static void > +reset_mirror_ctx(struct xlate_ctx *ctx, const struct flow *flow, > + const struct ofpact *a) > +{ > + switch (a->type) { > + case OFPACT_STRIP_VLAN: > + case OFPACT_PUSH_VLAN: > + case OFPACT_SET_ETH_SRC: > + case OFPACT_SET_ETH_DST: > + case OFPACT_PUSH_MPLS: > + case OFPACT_POP_MPLS: > + case OFPACT_SET_MPLS_LABEL: > + case OFPACT_SET_MPLS_TC: > + case OFPACT_SET_MPLS_TTL: > + case OFPACT_DEC_MPLS_TTL: > + case OFPACT_DEC_NSH_TTL: > + case OFPACT_DEC_TTL: > + case OFPACT_SET_VLAN_VID: > + case OFPACT_SET_VLAN_PCP: > + case OFPACT_ENCAP: > + case OFPACT_DECAP: > + case OFPACT_NAT: > + ctx->mirrors = 0; > + return; An empty line needed after the case body here and below. > + case OFPACT_SET_FIELD: { > + const struct ofpact_set_field *set_field; > + const struct mf_field *mf; > + > + set_field = ofpact_get_SET_FIELD(a); > + mf = set_field->field; > + if (mf_are_prereqs_ok(mf, flow, ctx->wc)) { We probably shouldn't change wildcards in this function. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
