diff mbox series

[ovs-dev,v2] ovn: Fix remote not receive GARP, when localnet Port has vlan tag.

Message ID 20170920151249.7476-1-ligs@dtdream.com
State Superseded
Headers show
Series [ovs-dev,v2] ovn: Fix remote not receive GARP, when localnet Port has vlan tag. | expand

Commit Message

Guoshuai Li Sept. 20, 2017, 3:12 p.m. UTC
When sending a localnet port with vlan, the GARP packet needs push_vlan.
---

v2: 
   Add check vlan vaid.
   Add update localnet vlan tag process.

---
 ovn/controller/pinctrl.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Miguel Angel Ajo Pelayo Sept. 21, 2017, 11:05 a.m. UTC | #1
The patch makes sense,

could we add some testing to make sure this is happening and ensure that we
don't hit regressions later?

On Wed, Sep 20, 2017 at 5:12 PM, Guoshuai Li <ligs@dtdream.com> wrote:

> When sending a localnet port with vlan, the GARP packet needs push_vlan.
> ---
>
> v2:
>    Add check vlan vaid.
>    Add update localnet vlan tag process.
>
> ---
>  ovn/controller/pinctrl.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 469a35586..eb9276833 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -1264,6 +1264,7 @@ struct garp_data {
>      long long int announce_time; /* Next announcement in ms. */
>      int backoff;                 /* Backoff for the next announcement. */
>      ofp_port_t ofport;           /* ofport used to output this GARP. */
> +    int64_t tag;                 /* vlan tag of this GARP packet. */
>  };
>
>  /* Contains GARPs to be sent. */
> @@ -1286,7 +1287,7 @@ destroy_send_garps(void)
>  }
>
>  static void
> -add_garp(const char *name, ofp_port_t ofport,
> +add_garp(const char *name, ofp_port_t ofport, int64_t tag,
>           const struct eth_addr ea, ovs_be32 ip)
>  {
>      struct garp_data *garp = xmalloc(sizeof *garp);
> @@ -1295,6 +1296,7 @@ add_garp(const char *name, ofp_port_t ofport,
>      garp->announce_time = time_msec() + 1000;
>      garp->backoff = 1;
>      garp->ofport = ofport;
> +    garp->tag = tag;
>      shash_add(&send_garp_data, name, garp);
>  }
>
> @@ -1313,6 +1315,8 @@ send_garp_update(const struct sbrec_port_binding
> *binding_rec,
>      }
>      ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports,
>                                               ld->localnet_port->logical_
> port));
> +    int64_t tag = ld->localnet_port->n_tag ?
> +                      *(ld->localnet_port->tag) : 0xFFFF;
>
>      volatile struct garp_data *garp = NULL;
>      /* Update GARP for NAT IP if it exists.  Consider port bindings with
> type
> @@ -1331,8 +1335,9 @@ send_garp_update(const struct sbrec_port_binding
> *binding_rec,
>                  garp = shash_find_data(&send_garp_data, name);
>                  if (garp) {
>                      garp->ofport = ofport;
> +                    garp->tag = tag;
>                  } else {
> -                    add_garp(name, ofport, laddrs->ea,
> +                    add_garp(name, ofport, tag, laddrs->ea,
>                               laddrs->ipv4_addrs[i].addr);
>                  }
>                  free(name);
> @@ -1359,7 +1364,7 @@ send_garp_update(const struct sbrec_port_binding
> *binding_rec,
>              continue;
>          }
>
> -        add_garp(binding_rec->logical_port, ofport,
> +        add_garp(binding_rec->logical_port, ofport, tag,
>                   laddrs.ea, laddrs.ipv4_addrs[0].addr);
>
>          destroy_lport_addresses(&laddrs);
> @@ -1389,6 +1394,11 @@ send_garp(struct garp_data *garp, long long int
> current_time)
>      compose_arp(&packet, ARP_OP_REQUEST, garp->ea, eth_addr_zero,
>                  true, garp->ipv4, garp->ipv4);
>
> +    /* Compose a GARP request packet's vlan if exist. */
> +    if (garp->tag >= 0 && garp->tag <= 4095) {
> +        eth_push_vlan(&packet, htons(ETH_TYPE_VLAN), htons(garp->tag));
> +    }
> +
>      /* Compose actions.  The garp request is output on localnet ofport. */
>      uint64_t ofpacts_stub[4096 / 8];
>      struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
> --
> 2.13.2.windows.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Russell Bryant Sept. 21, 2017, 11:47 p.m. UTC | #2
Yes, a test case would be great.

Can you also please add a "Signed-off-by" header to the commit
message?  For more information, see:
http://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/

On Thu, Sep 21, 2017 at 7:05 AM, Miguel Angel Ajo Pelayo
<majopela@redhat.com> wrote:
> The patch makes sense,
>
> could we add some testing to make sure this is happening and ensure that we
> don't hit regressions later?
>
> On Wed, Sep 20, 2017 at 5:12 PM, Guoshuai Li <ligs@dtdream.com> wrote:
>
>> When sending a localnet port with vlan, the GARP packet needs push_vlan.
>> ---
>>
>> v2:
>>    Add check vlan vaid.
>>    Add update localnet vlan tag process.
>>
>> ---
>>  ovn/controller/pinctrl.c | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
>> index 469a35586..eb9276833 100644
>> --- a/ovn/controller/pinctrl.c
>> +++ b/ovn/controller/pinctrl.c
>> @@ -1264,6 +1264,7 @@ struct garp_data {
>>      long long int announce_time; /* Next announcement in ms. */
>>      int backoff;                 /* Backoff for the next announcement. */
>>      ofp_port_t ofport;           /* ofport used to output this GARP. */
>> +    int64_t tag;                 /* vlan tag of this GARP packet. */
>>  };
>>
>>  /* Contains GARPs to be sent. */
>> @@ -1286,7 +1287,7 @@ destroy_send_garps(void)
>>  }
>>
>>  static void
>> -add_garp(const char *name, ofp_port_t ofport,
>> +add_garp(const char *name, ofp_port_t ofport, int64_t tag,
>>           const struct eth_addr ea, ovs_be32 ip)
>>  {
>>      struct garp_data *garp = xmalloc(sizeof *garp);
>> @@ -1295,6 +1296,7 @@ add_garp(const char *name, ofp_port_t ofport,
>>      garp->announce_time = time_msec() + 1000;
>>      garp->backoff = 1;
>>      garp->ofport = ofport;
>> +    garp->tag = tag;
>>      shash_add(&send_garp_data, name, garp);
>>  }
>>
>> @@ -1313,6 +1315,8 @@ send_garp_update(const struct sbrec_port_binding
>> *binding_rec,
>>      }
>>      ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports,
>>                                               ld->localnet_port->logical_
>> port));
>> +    int64_t tag = ld->localnet_port->n_tag ?
>> +                      *(ld->localnet_port->tag) : 0xFFFF;
>>
>>      volatile struct garp_data *garp = NULL;
>>      /* Update GARP for NAT IP if it exists.  Consider port bindings with
>> type
>> @@ -1331,8 +1335,9 @@ send_garp_update(const struct sbrec_port_binding
>> *binding_rec,
>>                  garp = shash_find_data(&send_garp_data, name);
>>                  if (garp) {
>>                      garp->ofport = ofport;
>> +                    garp->tag = tag;
>>                  } else {
>> -                    add_garp(name, ofport, laddrs->ea,
>> +                    add_garp(name, ofport, tag, laddrs->ea,
>>                               laddrs->ipv4_addrs[i].addr);
>>                  }
>>                  free(name);
>> @@ -1359,7 +1364,7 @@ send_garp_update(const struct sbrec_port_binding
>> *binding_rec,
>>              continue;
>>          }
>>
>> -        add_garp(binding_rec->logical_port, ofport,
>> +        add_garp(binding_rec->logical_port, ofport, tag,
>>                   laddrs.ea, laddrs.ipv4_addrs[0].addr);
>>
>>          destroy_lport_addresses(&laddrs);
>> @@ -1389,6 +1394,11 @@ send_garp(struct garp_data *garp, long long int
>> current_time)
>>      compose_arp(&packet, ARP_OP_REQUEST, garp->ea, eth_addr_zero,
>>                  true, garp->ipv4, garp->ipv4);
>>
>> +    /* Compose a GARP request packet's vlan if exist. */
>> +    if (garp->tag >= 0 && garp->tag <= 4095) {
>> +        eth_push_vlan(&packet, htons(ETH_TYPE_VLAN), htons(garp->tag));
>> +    }
>> +
>>      /* Compose actions.  The garp request is output on localnet ofport. */
>>      uint64_t ofpacts_stub[4096 / 8];
>>      struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
>> --
>> 2.13.2.windows.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 469a35586..eb9276833 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1264,6 +1264,7 @@  struct garp_data {
     long long int announce_time; /* Next announcement in ms. */
     int backoff;                 /* Backoff for the next announcement. */
     ofp_port_t ofport;           /* ofport used to output this GARP. */
+    int64_t tag;                 /* vlan tag of this GARP packet. */
 };
 
 /* Contains GARPs to be sent. */
@@ -1286,7 +1287,7 @@  destroy_send_garps(void)
 }
 
 static void
-add_garp(const char *name, ofp_port_t ofport,
+add_garp(const char *name, ofp_port_t ofport, int64_t tag,
          const struct eth_addr ea, ovs_be32 ip)
 {
     struct garp_data *garp = xmalloc(sizeof *garp);
@@ -1295,6 +1296,7 @@  add_garp(const char *name, ofp_port_t ofport,
     garp->announce_time = time_msec() + 1000;
     garp->backoff = 1;
     garp->ofport = ofport;
+    garp->tag = tag;
     shash_add(&send_garp_data, name, garp);
 }
 
@@ -1313,6 +1315,8 @@  send_garp_update(const struct sbrec_port_binding *binding_rec,
     }
     ofp_port_t ofport = u16_to_ofp(simap_get(localnet_ofports,
                                              ld->localnet_port->logical_port));
+    int64_t tag = ld->localnet_port->n_tag ? 
+                      *(ld->localnet_port->tag) : 0xFFFF;
 
     volatile struct garp_data *garp = NULL;
     /* Update GARP for NAT IP if it exists.  Consider port bindings with type
@@ -1331,8 +1335,9 @@  send_garp_update(const struct sbrec_port_binding *binding_rec,
                 garp = shash_find_data(&send_garp_data, name);
                 if (garp) {
                     garp->ofport = ofport;
+                    garp->tag = tag;
                 } else {
-                    add_garp(name, ofport, laddrs->ea,
+                    add_garp(name, ofport, tag, laddrs->ea,
                              laddrs->ipv4_addrs[i].addr);
                 }
                 free(name);
@@ -1359,7 +1364,7 @@  send_garp_update(const struct sbrec_port_binding *binding_rec,
             continue;
         }
 
-        add_garp(binding_rec->logical_port, ofport,
+        add_garp(binding_rec->logical_port, ofport, tag,
                  laddrs.ea, laddrs.ipv4_addrs[0].addr);
 
         destroy_lport_addresses(&laddrs);
@@ -1389,6 +1394,11 @@  send_garp(struct garp_data *garp, long long int current_time)
     compose_arp(&packet, ARP_OP_REQUEST, garp->ea, eth_addr_zero,
                 true, garp->ipv4, garp->ipv4);
 
+    /* Compose a GARP request packet's vlan if exist. */
+    if (garp->tag >= 0 && garp->tag <= 4095) {
+        eth_push_vlan(&packet, htons(ETH_TYPE_VLAN), htons(garp->tag));
+    }
+
     /* Compose actions.  The garp request is output on localnet ofport. */
     uint64_t ofpacts_stub[4096 / 8];
     struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);