On Thu, May 04, 2017 at 08:43:42PM +0530, [email protected] wrote:
> From: Zong Kai LI <[email protected]>
>
> This patchs add a new action 'nd_ra' against current 'nd_na' action, to work
> as
> a Router Solicitation packet responder. The inner actions to set fields,
> flags,
> options in RA packet will be introduced in the following patch.
>
> Signed-off-by: Zongkai LI <[email protected]>
> Signed-off-by: Numan Siddique <[email protected]>
Thanks for working on this.
"sparse" complains about the variable-length array here:
bool as_nd_na = ip_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT);
int packet_stub_size = as_nd_na ? 128 : 256;
uint64_t packet_stub[packet_stub_size / 8];
and indeed I don't think that it's necessary. If you think that RS
messages are likely to be bigger than 1 kB, just change the 128 to 256;
there is no need to conserve memory here.
In ovn-trace, I noticed that the new case was going to assert-fail
because it applied ovnact_get_ND_NA to a different kind of action.
ovn.at needed a minor revision since the dns_lookup action is now
available.
I'm appending an incremental patch that fixes these issues and makes
some style improvements also.
--8<--------------------------cut here-------------------------->8--
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 31b07f5f6685..e5ed3371742a 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1756,13 +1756,11 @@ pinctrl_handle_nd(const struct flow *ip_flow, const
struct match *md,
enum ofp_version version = rconn_get_version(swconn);
enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
- bool as_nd_na = ip_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT);
- int packet_stub_size = as_nd_na ? 128 : 256;
- uint64_t packet_stub[packet_stub_size / 8];
+ uint64_t packet_stub[256 / 8];
struct dp_packet packet;
dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
- if (as_nd_na) {
+ if (ip_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT)) {
/* xxx These flags are not exactly correct. Look at section 7.2.4
* xxx of RFC 4861. For example, we need to set ND_RSO_ROUTER for
* xxx router's interfaces and ND_RSO_SOLICITED only if it was
@@ -1775,16 +1773,15 @@ pinctrl_handle_nd(const struct flow *ip_flow, const
struct match *md,
* xxx The following patch should fix this. */
uint8_t cur_hop_limit = 64;
uint8_t mo_flags = 0;
- ovs_be16 router_lifetime = 0xffff;
+ ovs_be16 router_lifetime = OVS_BE16_MAX;
ovs_be32 reachable_time = 0;
ovs_be32 retrans_timer = 0;
ovs_be32 mtu = 0;
- compose_nd_ra_with_sll_mtu_opts(
- &packet, ip_flow->dl_dst, ip_flow->dl_src,
- &ip_flow->ipv6_dst, &ip_flow->ipv6_src,
- cur_hop_limit, mo_flags, router_lifetime, reachable_time,
- retrans_timer, mtu);
+ compose_nd_ra(&packet, ip_flow->dl_dst, ip_flow->dl_src,
+ &ip_flow->ipv6_dst, &ip_flow->ipv6_src,
+ cur_hop_limit, mo_flags, router_lifetime, reachable_time,
+ retrans_timer, mtu);
}
/* Reload previous packet metadata. */
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index d361c0ec73eb..38b2863fb1e0 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1131,15 +1131,15 @@ parse_ND_NA(struct action_context *ctx)
}
static void
-parse_CLONE(struct action_context *ctx)
+parse_ND_RA(struct action_context *ctx)
{
- parse_nested_action(ctx, OVNACT_CLONE, NULL);
+ parse_nested_action(ctx, OVNACT_ND_RA, "nd_rs");
}
static void
-parse_ND_RA(struct action_context *ctx)
+parse_CLONE(struct action_context *ctx)
{
- parse_nested_action(ctx, OVNACT_ND_RA, "nd_rs");
+ parse_nested_action(ctx, OVNACT_CLONE, NULL);
}
static void
@@ -1216,6 +1216,14 @@ encode_ND_NA(const struct ovnact_nest *on,
}
static void
+encode_ND_RA(const struct ovnact_nest *on,
+ const struct ovnact_encode_params *ep,
+ struct ofpbuf *ofpacts)
+{
+ encode_nested_neighbor_actions(on, ep, ACTION_OPCODE_ND_RA, ofpacts);
+}
+
+static void
encode_CLONE(const struct ovnact_nest *on,
const struct ovnact_encode_params *ep,
struct ofpbuf *ofpacts)
@@ -1230,14 +1238,6 @@ encode_CLONE(const struct ovnact_nest *on,
}
static void
-encode_ND_RA(const struct ovnact_nest *on,
- const struct ovnact_encode_params *ep,
- struct ofpbuf *ofpacts)
-{
- encode_nested_neighbor_actions(on, ep, ACTION_OPCODE_ND_RA, ofpacts);
-}
-
-static void
ovnact_nest_free(struct ovnact_nest *on)
{
ovnacts_free(on->nested, on->nested_len);
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index dd959fa8d638..8eeb737423f5 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -1499,29 +1499,30 @@ execute_arp(const struct ovnact_nest *on, const struct
ovntrace_datapath *dp,
table_id, pipeline, &node->subs);
}
+/* Executes nd_na and nd_ra actions. */
static void
-execute_nd_na(const struct ovnact_nest *on, const struct ovntrace_datapath *dp,
+execute_nd_xa(const struct ovnact_nest *on, const struct ovntrace_datapath *dp,
const struct flow *uflow, uint8_t table_id,
- enum ovnact_pipeline pipeline, struct ovs_list *super,
- bool is_neighbor_adv)
+ enum ovnact_pipeline pipeline, struct ovs_list *super)
{
- struct flow na_flow = *uflow;
-
- /* Update fields for ND Router/Neighbor Advertisement. */
- na_flow.dl_src = uflow->dl_dst;
- na_flow.dl_dst = uflow->dl_src;
- na_flow.ipv6_dst = uflow->ipv6_src;
- na_flow.ipv6_src = uflow->nd_target;
- na_flow.tp_src = is_neighbor_adv ? htons(136) : htons(134);
- na_flow.arp_sha = eth_addr_zero;
- na_flow.arp_tha = uflow->dl_dst;
+ bool is_na = on->ovnact.type == OVNACT_ND_NA;
+ bool icmp6_type = is_na ? 136 : 134;
+ const char *name = is_na ? "nd_na" : "nd_ra";
+
+ struct flow xa_flow = *uflow;
+ xa_flow.dl_src = uflow->dl_dst;
+ xa_flow.dl_dst = uflow->dl_src;
+ xa_flow.ipv6_dst = uflow->ipv6_src;
+ xa_flow.ipv6_src = uflow->nd_target;
+ xa_flow.tp_src = htons(icmp6_type);
+ xa_flow.arp_sha = eth_addr_zero;
+ xa_flow.arp_tha = uflow->dl_dst;
struct ovntrace_node *node = ovntrace_node_append(
- super, OVNTRACE_NODE_TRANSFORMATION,
- (is_neighbor_adv ? "nd_na" : "nd_ra"));
+ super, OVNTRACE_NODE_TRANSFORMATION, "%s", name);
- trace_actions(on->nested, on->nested_len, dp, &na_flow,
+ trace_actions(on->nested, on->nested_len, dp, &xa_flow,
table_id, pipeline, &node->subs);
}
@@ -1795,9 +1796,13 @@ trace_actions(const struct ovnact *ovnacts, size_t
ovnacts_len,
break;
case OVNACT_ND_NA:
- case OVNACT_ND_RA:
- execute_nd_na(ovnact_get_ND_NA(a), dp, uflow, table_id, pipeline,
- super, (a->type == OVNACT_ND_NA));
+ execute_nd_xa(ovnact_get_ND_NA(a), dp, uflow, table_id, pipeline,
+ super);
+ break;
+
+ case OVNACT_ND_RA:
+ execute_nd_xa(ovnact_get_ND_RA(a), dp, uflow, table_id, pipeline,
+ super);
break;
case OVNACT_GET_ARP:
diff --git a/tests/ovn.at b/tests/ovn.at
index 807161aabda5..b9cc63247b87 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -970,7 +970,7 @@ nd_na { eth.src = 12:34:56:78:9a:bc; nd.tll =
12:34:56:78:9a:bc; outport = inpor
# nd_ra
nd_ra{eth.src = 12:34:56:78:9a:bc; ip6.src = fdad:1234:5678::1; outport =
inport; flags.loopback = 1; output;};
formats as nd_ra { eth.src = 12:34:56:78:9a:bc; ip6.src =
fdad:1234:5678::1; outport = inport; flags.loopback = 1; output; };
- encodes as
controller(userdata=00.00.00.06.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.18.80.00.34.10.fd.ad.12.34.56.78.00.00.00.00.00.00.00.00.00.01.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.1c.04.00.01.1e.04.ff.ff.00.18.00.00.23.20.00.07.00.00.00.01.14.04.00.00.00.00.00.00.00.01.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
+ encodes as
controller(userdata=00.00.00.07.00.00.00.00.00.19.00.10.80.00.08.06.12.34.56.78.9a.bc.00.00.00.19.00.18.80.00.34.10.fd.ad.12.34.56.78.00.00.00.00.00.00.00.00.00.01.ff.ff.00.18.00.00.23.20.00.06.00.20.00.00.00.00.00.01.1c.04.00.01.1e.04.ff.ff.00.18.00.00.23.20.00.07.00.00.00.01.14.04.00.00.00.00.00.00.00.01.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
has prereqs nd_rs
# get_nd
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
