[ovs-dev] ip_src_flow

[Ben Pfaff]

On Fri, Aug 23, 2013 at 12:46:47PM -0700, Jarno Rajahalme wrote:
> On Aug 23, 2013, at 11:26 AM, Ben Pfaff <blp at nicira.com> wrote:
> > Looking at tnl_find() in ofproto/tunnel.c, it maintains the invariant
> > "set <field> to 0 if and only if we set <field>_flow to true", except
> > for <field> = ip_src.  Is that a bug?
> > 
> 
> No, it is not a bug. Tunnel config allows you to leave local address
> unset (=0), allowing the selected route to determine the source
> address to use. Even more, it looks like find_route() in kernel
> might override the source address, even if it is given (in the
> config, or in the flow with _flow set to true).

I see, thanks.

The code might be more obviously correct like this.  If anyone agrees
then I'll check it over and write it up formally.

diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 202358b..1559b16 100644
*** a/ofproto/tunnel.c
--- b/ofproto/tunnel.c
***************
*** 416,466 ****
  static struct tnl_port *
  tnl_find(struct tnl_match *match_) OVS_REQ_RDLOCK(rwlock)
  {
!     struct tnl_match match = *match_;
!     struct tnl_port *tnl_port;
  
!     /* remote_ip, local_ip, in_key */
!     tnl_port = tnl_find_exact(&match);
!     if (tnl_port) {
!         return tnl_port;
!     }
  
!     /* 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;
  
!     /* 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;
!     }
  
!     /* remote_ip */
!     match.ip_src = 0;
!     tnl_port = tnl_find_exact(&match);
!     if (tnl_port) {
!         return tnl_port;
!     }
  
!     /* Flow-based remote */
!     match.ip_dst = 0;
!     match.ip_dst_flow = true;
!     tnl_port = tnl_find_exact(&match);
!     if (tnl_port) {
!         return tnl_port;
!     }
  
!     /* Flow-based everything */
!     match.ip_src_flow = true;
!     tnl_port = tnl_find_exact(&match);
!     if (tnl_port) {
!         return tnl_port;
      }
  
      return NULL;
--- 416,461 ----
  static struct tnl_port *
  tnl_find(struct tnl_match *match_) OVS_REQ_RDLOCK(rwlock)
  {
!     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. */
!     };
  
!     struct tnl_match match = *match_;
!     const struct tnl_match_pattern *p;
  
!     for (p = patterns; p < &patterns[ARRAY_SIZE(patterns)]; p++) {
!         struct tnl_port *tnl_port;
  
!         match.in_key_flow = p->in_key_flow;
!         match.in_key = p->in_key_flow ? 0 : match_->in_key;
  
!         match.ip_dst_flow = p->ip_dst_flow;
!         match.ip_dst = p->ip_dst_flow ? 0 : match_->ip_dst_flow;
  
!         match.ip_src_flow = p->ip_src == IP_SRC_FLOW;
!         match.ip_src = p->ip_src == IP_SRC_EXACT ? match_->ip_src : 0;
  
!         tnl_port = tnl_find_exact(&match);
!         if (tnl_port) {
!             return tnl_port;
!         }
      }
  
      return NULL;