[ovs-dev] [PATCH] tunnel: Make tnl_find() easier to understand.

Ben Pfaff blp at nicira.com
Tue Aug 27 19:19:13 UTC 2013


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.kothari at cisco.com>
> 
> On Aug 27, 2013, at 11:12 AM, Ben Pfaff wrote:
> 
> > Suggested-by: pritesh <pritesh.kothari at cisco.com>
> > Signed-off-by: Ben Pfaff <blp at 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
> > 
> 



More information about the dev mailing list