On Tue, Nov 20, 2012 at 10:15 AM, Pravin B Shelar <[email protected]> wrote:
> Signed-off-by: Pravin B Shelar <[email protected]>
> ---
> lib/flow.c | 30 ++++++++
> lib/flow.h | 19 +++++
> lib/match.c | 44 ++++++++---
> lib/meta-flow.c | 211
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/meta-flow.h | 13 +++-
> lib/odp-util.c | 45 ++---------
> tests/ovs-ofctl.at | 2 +
> 7 files changed, 310 insertions(+), 54 deletions(-)
Ben, I think you probably want to review the metaflow stuff. For
reference, these tunnel parameters are being printed as part of logs
and we want to be able to take them through trace but they are aren't
matchable through OpenFlow.
> diff --git a/lib/match.c b/lib/match.c
> index 81b7173..bf595c9 100644
> --- a/lib/match.c
> +++ b/lib/match.c
In addition to the tunnel fields, we should also add skb_priority to
be printed/parsed. Also, Ansis is about to add skb_mark, which will
also fall in this category, so whoever's patch goes in second should
add it here as well.
We should also check that the conditions for tunneling are right in
match_wc_init(). Currently, if either ip_dst or tun_id are set then
it considers everything to be significant. If this is correct then we
can simplify it into one memset for the entire struct. However, we
should see if we should break them apart into separate tun_id and rest
of struct masking.
> +static void
> +flow_tunnel_format(const struct match *match, struct ds *s)
> +{
> + const struct flow_wildcards *wc = &match->wc;
> + const struct flow_tnl *tnl = &match->flow.tunnel;
> +
> + switch (wc->masks.tunnel.tun_id) {
> + case 0:
> + return;
> + case CONSTANT_HTONLL(UINT64_MAX):
> + ds_put_format(s, "tun_id=%#"PRIx64, ntohll(tnl->tun_id));
> + break;
> + default:
> + ds_put_format(s, "tun_id=%#"PRIx64"/%#"PRIx64,
> + ntohll(tnl->tun_id),
> + ntohll(wc->masks.tunnel.tun_id));
> + break;
> + }
> + if (tnl->ip_dst) {
> + ds_put_format(s, ",tun_src="IP_FMT",tun_dst="IP_FMT","
> + "tun_tos=0x%"PRIx8",tun_ttl=%"PRIu8",tun_flags",
> + IP_ARGS(&tnl->ip_src),
> + IP_ARGS(&tnl->ip_dst),
> + tnl->ip_tos, tnl->ip_ttl);
> + format_flags(s, flow_tun_flag_to_string, tnl->flags, '.');
> + }
> + ds_put_char(s, ',');
> +}
I think we probably want to rely on the masks more when deciding what
to print here. Right now the conditions under which we can have
tun_id and the ip_dst are closely tied but let's put that logic in the
place where compute the mask rather than where we print it. We also
probably shouldn't return if tun_id is wildcarded.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev