Thanks.
I am going to give others a day or so to look at this, since it is
only a cleanup. I'll apply it then assuming I get no negative
feedback.
On Tue, Aug 27, 2013 at 07:01:11PM +0000, Pritesh Kothari (pritkoth) wrote:
> Looks good and simple to understand. (passed tests also).
>
> Acked-by: pritesh <pritesh.koth...@cisco.com>
>
> On Aug 27, 2013, at 11:12 AM, Ben Pfaff wrote:
>
> > Suggested-by: pritesh <pritesh.koth...@cisco.com>
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > ofproto/tunnel.c | 100
> > +++++++++++++++++++++++++-----------------------------
> > 1 file changed, 46 insertions(+), 54 deletions(-)
> >
> > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> > index 202358b..4729f5f 100644
> > --- a/ofproto/tunnel.c
> > +++ b/ofproto/tunnel.c
> > @@ -67,7 +67,7 @@ static struct hmap *ofport_map OVS_GUARDED_BY(rwlock) =
> > &ofport_map__;
> > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> > static struct vlog_rate_limit dbg_rl = VLOG_RATE_LIMIT_INIT(60, 60);
> >
> > -static struct tnl_port *tnl_find(struct tnl_match *)
> > OVS_REQ_RDLOCK(rwlock);
> > +static struct tnl_port *tnl_find(const struct flow *)
> > OVS_REQ_RDLOCK(rwlock);
> > static struct tnl_port *tnl_find_exact(struct tnl_match *)
> > OVS_REQ_RDLOCK(rwlock);
> > static struct tnl_port *tnl_find_ofport(const struct ofport_dpif *)
> > @@ -209,24 +209,15 @@ tnl_port_receive(const struct flow *flow)
> > OVS_EXCLUDED(rwlock)
> > char *pre_flow_str = NULL;
> > const struct ofport_dpif *ofport;
> > struct tnl_port *tnl_port;
> > - struct tnl_match match;
> > -
> > - memset(&match, 0, sizeof match);
> > - match.odp_port = flow->in_port.odp_port;
> > - match.ip_src = flow->tunnel.ip_dst;
> > - match.ip_dst = flow->tunnel.ip_src;
> > - match.in_key = flow->tunnel.tun_id;
> > - match.pkt_mark = flow->pkt_mark;
> >
> > ovs_rwlock_rdlock(&rwlock);
> > - tnl_port = tnl_find(&match);
> > + tnl_port = tnl_find(flow);
> > ofport = tnl_port ? tnl_port->ofport : NULL;
> > if (!tnl_port) {
> > - struct ds ds = DS_EMPTY_INITIALIZER;
> > + char *flow_str = flow_to_string(flow);
> >
> > - tnl_match_fmt(&match, &ds);
> > - VLOG_WARN_RL(&rl, "receive tunnel port not found (%s)",
> > ds_cstr(&ds));
> > - ds_destroy(&ds);
> > + VLOG_WARN_RL(&rl, "receive tunnel port not found (%s)", flow_str);
> > + free(flow_str);
> > goto out;
> > }
> >
> > @@ -413,54 +404,55 @@ tnl_find_exact(struct tnl_match *match)
> > OVS_REQ_RDLOCK(rwlock)
> > return NULL;
> > }
> >
> > +/* Returns the tnl_port that is the best match for the tunnel data in
> > 'flow',
> > + * or NULL if no tnl_port matches 'flow'. */
> > static struct tnl_port *
> > -tnl_find(struct tnl_match *match_) OVS_REQ_RDLOCK(rwlock)
> > +tnl_find(const struct flow *flow) OVS_REQ_RDLOCK(rwlock)
> > {
> > - struct tnl_match match = *match_;
> > - struct tnl_port *tnl_port;
> > + enum ip_src_type {
> > + IP_SRC_EXACT, /* ip_src must match exactly. */
> > + IP_SRC_ANY, /* Any ip_src is acceptable. */
> > + IP_SRC_FLOW /* ip_src is handled in flow table. */
> > + };
> > +
> > + struct tnl_match_pattern {
> > + bool in_key_flow;
> > + bool ip_dst_flow;
> > + enum ip_src_type ip_src;
> > + };
> > +
> > + static const struct tnl_match_pattern patterns[] = {
> > + { false, false, IP_SRC_EXACT }, /* remote_ip, local_ip, in_key. */
> > + { false, false, IP_SRC_ANY }, /* remote_ip, in_key. */
> > + { true, false, IP_SRC_EXACT }, /* remote_ip, local_ip. */
> > + { true, false, IP_SRC_ANY }, /* remote_ip. */
> > + { true, true, IP_SRC_ANY }, /* Flow-based remote. */
> > + { true, true, IP_SRC_FLOW }, /* Flow-based everything. */
> > + };
> > +
> > + const struct tnl_match_pattern *p;
> > + struct tnl_match match;
> >
> > - /* remote_ip, local_ip, in_key */
> > - tnl_port = tnl_find_exact(&match);
> > - if (tnl_port) {
> > - return tnl_port;
> > - }
> > + memset(&match, 0, sizeof match);
> > + match.odp_port = flow->in_port.odp_port;
> > + match.pkt_mark = flow->pkt_mark;
> >
> > - /* remote_ip, in_key */
> > - match.ip_src = 0;
> > - tnl_port = tnl_find_exact(&match);
> > - if (tnl_port) {
> > - return tnl_port;
> > - }
> > - match.ip_src = match_->ip_src;
> > + for (p = patterns; p < &patterns[ARRAY_SIZE(patterns)]; p++) {
> > + struct tnl_port *tnl_port;
> >
> > - /* remote_ip, local_ip */
> > - match.in_key = 0;
> > - match.in_key_flow = true;
> > - tnl_port = tnl_find_exact(&match);
> > - if (tnl_port) {
> > - return tnl_port;
> > - }
> > + match.in_key_flow = p->in_key_flow;
> > + match.in_key = p->in_key_flow ? 0 : flow->tunnel.tun_id;
> >
> > - /* remote_ip */
> > - match.ip_src = 0;
> > - tnl_port = tnl_find_exact(&match);
> > - if (tnl_port) {
> > - return tnl_port;
> > - }
> > + match.ip_dst_flow = p->ip_dst_flow;
> > + match.ip_dst = p->ip_dst_flow ? 0 : flow->tunnel.ip_src;
> >
> > - /* Flow-based remote */
> > - match.ip_dst = 0;
> > - match.ip_dst_flow = true;
> > - tnl_port = tnl_find_exact(&match);
> > - if (tnl_port) {
> > - return tnl_port;
> > - }
> > + match.ip_src_flow = p->ip_src == IP_SRC_FLOW;
> > + match.ip_src = p->ip_src == IP_SRC_EXACT ? flow->tunnel.ip_dst : 0;
> >
> > - /* Flow-based everything */
> > - match.ip_src_flow = true;
> > - tnl_port = tnl_find_exact(&match);
> > - if (tnl_port) {
> > - return tnl_port;
> > + tnl_port = tnl_find_exact(&match);
> > + if (tnl_port) {
> > + return tnl_port;
> > + }
> > }
> >
> > return NULL;
> > --
> > 1.7.10.4
> >
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
