On Tue, Jul 7, 2020 at 8:49 PM Lorenzo Bianconi <[email protected]> wrote:
> Intoduce icmp6_error action in order to generate an ICMPv6 packet in > response to an error in original IPv6 packet > > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > Hi Lorenzo, Thanks for v3. I just have one small nit. You need to update a few comments at the beginning of pinctrl_handle_put_icmp_frag_mtu() function. ***** diff --git a/controller/pinctrl.c b/controller/pinctrl.c index f07fc92c8..4ad3d1fb1 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -5477,10 +5477,10 @@ pinctrl_handle_put_icmp_frag_mtu(struct rconn *swconn, enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version); struct dp_packet *pkt_out = NULL; - /* This action only works for ICMPv4 packets. */ + /* This action only works for ICMPv4/v6 packets. */ if (!is_icmpv4(in_flow, NULL) && !is_icmpv6(in_flow, NULL)) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); - VLOG_WARN_RL(&rl, "put_icmp4_frag_mtu action on non-ICMPv4 packet"); + VLOG_WARN_RL(&rl, "put_icmp(4/6)_frag_mtu action on non-ICMPv4/v6 packet"); goto exit; } **** I was thinking to update this patch myself and apply the series. But then I've a comment in p3 and I think it's better to address that. Thanks Numan controller/pinctrl.c | 1 + > include/ovn/actions.h | 9 ++++++++- > lib/actions.c | 22 ++++++++++++++++++++++ > ovn-sb.xml | 8 ++++++++ > tests/ovn.at | 10 ++++++++++ > utilities/ovn-trace.c | 5 +++++ > 6 files changed, 54 insertions(+), 1 deletion(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index b2c656efb..61dad9752 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -2772,6 +2772,7 @@ process_packet_in(struct rconn *swconn, const struct > ofp_header *msg) > break; > > case ACTION_OPCODE_ICMP4_ERROR: > + case ACTION_OPCODE_ICMP6_ERROR: > pinctrl_handle_icmp(swconn, &headers, &packet, &pin.flow_metadata, > &userdata, false); > break; > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index 4a54abe17..652873676 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -92,7 +92,8 @@ struct ovn_extend_table; > OVNACT(BIND_VPORT, ovnact_bind_vport) \ > OVNACT(HANDLE_SVC_CHECK, ovnact_handle_svc_check) \ > OVNACT(FWD_GROUP, ovnact_fwd_group) \ > - OVNACT(DHCP6_REPLY, ovnact_null) > + OVNACT(DHCP6_REPLY, ovnact_null) \ > + OVNACT(ICMP6_ERROR, ovnact_nest) > > /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */ > enum OVS_PACKED_ENUM ovnact_type { > @@ -591,6 +592,12 @@ enum action_opcode { > * The actions, in OpenFlow 1.3 format, follow the action_header. > */ > ACTION_OPCODE_DHCP6_SERVER, > + > + /* "icmp6_error { ...actions... }". > + * > + * The actions, in OpenFlow 1.3 format, follow the action_header. > + */ > + ACTION_OPCODE_ICMP6_ERROR, > }; > > /* Header. */ > diff --git a/lib/actions.c b/lib/actions.c > index cd086d2fa..6a3b1ba87 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -1408,6 +1408,12 @@ parse_ICMP6(struct action_context *ctx) > parse_nested_action(ctx, OVNACT_ICMP6, "ip6"); > } > > +static void > +parse_ICMP6_ERROR(struct action_context *ctx) > +{ > + parse_nested_action(ctx, OVNACT_ICMP6_ERROR, "ip6"); > +} > + > static void > parse_TCP_RESET(struct action_context *ctx) > { > @@ -1471,6 +1477,12 @@ format_ICMP6(const struct ovnact_nest *nest, struct > ds *s) > format_nested_action(nest, "icmp6", s); > } > > +static void > +format_ICMP6_ERROR(const struct ovnact_nest *nest, struct ds *s) > +{ > + format_nested_action(nest, "icmp6_error", s); > +} > + > static void > format_IGMP(const struct ovnact_null *a OVS_UNUSED, struct ds *s) > { > @@ -1582,6 +1594,14 @@ encode_ICMP6(const struct ovnact_nest *on, > encode_nested_actions(on, ep, ACTION_OPCODE_ICMP, ofpacts); > } > > +static void > +encode_ICMP6_ERROR(const struct ovnact_nest *on, > + const struct ovnact_encode_params *ep, > + struct ofpbuf *ofpacts) > +{ > + encode_nested_actions(on, ep, ACTION_OPCODE_ICMP6_ERROR, ofpacts); > +} > + > static void > encode_IGMP(const struct ovnact_null *a OVS_UNUSED, > const struct ovnact_encode_params *ep OVS_UNUSED, > @@ -3447,6 +3467,8 @@ parse_action(struct action_context *ctx) > parse_ICMP4_ERROR(ctx); > } else if (lexer_match_id(ctx->lexer, "icmp6")) { > parse_ICMP6(ctx); > + } else if (lexer_match_id(ctx->lexer, "icmp6_error")) { > + parse_ICMP6_ERROR(ctx); > } else if (lexer_match_id(ctx->lexer, "igmp")) { > ovnact_put_IGMP(ctx->ovnacts); > } else if (lexer_match_id(ctx->lexer, "tcp_reset")) { > diff --git a/ovn-sb.xml b/ovn-sb.xml > index cf95a7c9c..bc69b58c0 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -2035,6 +2035,9 @@ > </dd> > > <dt><code>icmp6 { <var>action</var>; </code>...<code> > };</code></dt> > + <dt> > + <code>icmp6_error { <var>action</var>; </code>...<code> > };</code> > + </dt> > <dd> > <p> > Temporarily replaces the IPv6 packet being processed by an > ICMPv6 > @@ -2057,6 +2060,11 @@ > <li><code>icmp6.code = 1</code> (administratively > prohibited)</li> > </ul> > > + <p> > + <code>icmp6_error</code> action is expected to be used to > + generate an ICMPv6 packet in response to an error in > original > + IPv6 packet. > + </p> > <p><b>Prerequisite:</b> <code>ip6</code></p> > </dd> > > diff --git a/tests/ovn.at b/tests/ovn.at > index 9522b55f8..30ee27ae9 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1500,6 +1500,16 @@ icmp6 { }; > encodes as controller(userdata=00.00.00.0a.00.00.00.00) > has prereqs ip6 > > +# icmp6_error > +icmp6_error { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output; > + encodes as > controller(userdata=00.00.00.14.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64) > + has prereqs ip6 > + > +icmp6_error { }; > + formats as icmp6_error { drop; }; > + encodes as controller(userdata=00.00.00.14.00.00.00.00) > + has prereqs ip6 > + > # tcp_reset > tcp_reset { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output; > encodes as > controller(userdata=00.00.00.0b.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64) > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > index 2666c1062..d1ab051ce 100644 > --- a/utilities/ovn-trace.c > +++ b/utilities/ovn-trace.c > @@ -2270,6 +2270,11 @@ trace_actions(const struct ovnact *ovnacts, size_t > ovnacts_len, > super); > break; > > + case OVNACT_ICMP6_ERROR: > + execute_icmp6(ovnact_get_ICMP6_ERROR(a), dp, uflow, table_id, > + pipeline, super); > + break; > + > case OVNACT_IGMP: > /* Nothing to do for tracing. */ > break; > -- > 2.26.2 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
