diff mbox series

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

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

Commit Message

Guoshuai Li Oct. 13, 2017, 2:09 p.m. UTC 
When sending a localnet port with vlan, the GARP packet needs push_vlan.

Signed-off-by: Guoshuai Li <ligs@dtdream.com>
---

v3:
   Add garp-vlan test.

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

---
 ovn/controller/pinctrl.c | 16 +++++++++++++---
 tests/ovn.at             | 31 +++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

Comments

Ben Pfaff Nov. 3, 2017, 9:41 p.m. UTC | #1 
On Fri, Oct 13, 2017 at 10:09:51PM +0800, Guoshuai Li wrote:
> When sending a localnet port with vlan, the GARP packet needs push_vlan.
> 
> Signed-off-by: Guoshuai Li <ligs@dtdream.com>
> ---
> 
> v3:
>    Add garp-vlan test.
> 
> v2:
>    Add check vlan vaid.
>    Add update localnet vlan tag process.

Thanks a lot.  I applied this to master.  (I did change the type of
'tag' from int64_t to just int.)
Ben Pfaff Nov. 3, 2017, 9:42 p.m. UTC | #2 
On Fri, Nov 03, 2017 at 02:41:54PM -0700, Ben Pfaff wrote:
> On Fri, Oct 13, 2017 at 10:09:51PM +0800, Guoshuai Li wrote:
> > When sending a localnet port with vlan, the GARP packet needs push_vlan.
> > 
> > Signed-off-by: Guoshuai Li <ligs@dtdream.com>
> > ---
> > 
> > v3:
> >    Add garp-vlan test.
> > 
> > v2:
> >    Add check vlan vaid.
> >    Add update localnet vlan tag process.
> 
> Thanks a lot.  I applied this to master.  (I did change the type of
> 'tag' from int64_t to just int.)

Also, if this is causing problems for older versions of OVN, let me know
and I'll cherry-pick it to an older branch.
Guoshuai Li Nov. 13, 2017, 10:30 a.m. UTC | #3 
> On Fri, Nov 03, 2017 at 02:41:54PM -0700, Ben Pfaff wrote:
>> On Fri, Oct 13, 2017 at 10:09:51PM +0800, Guoshuai Li wrote:
>>> When sending a localnet port with vlan, the GARP packet needs push_vlan.
>>>
>>> Signed-off-by: Guoshuai Li <ligs@dtdream.com>
>>> ---
>>>
>>> v3:
>>>     Add garp-vlan test.
>>>
>>> v2:
>>>     Add check vlan vaid.
>>>     Add update localnet vlan tag process.
>> Thanks a lot.  I applied this to master.  (I did change the type of
>> 'tag' from int64_t to just int.)
> Also, if this is causing problems for older versions of OVN, let me know
> and I'll cherry-pick it to an older branch.

hi, ben. I am sorry for the late reply, a few days ago on leave.

Since 2.6 branche, this problem exists.
So I think it need cherry-pick to branch-2.8/branch-2.7/branch-2.6

Cherry-pick to branch-2.7/branch-2.6 is conflicted,  I sent a new commit:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/340854.html
Ben Pfaff Nov. 13, 2017, 4:49 p.m. UTC | #4 
On Mon, Nov 13, 2017 at 06:30:02PM +0800, Guoshuai Li wrote:
> 
> 
> >On Fri, Nov 03, 2017 at 02:41:54PM -0700, Ben Pfaff wrote:
> >>On Fri, Oct 13, 2017 at 10:09:51PM +0800, Guoshuai Li wrote:
> >>>When sending a localnet port with vlan, the GARP packet needs push_vlan.
> >>>
> >>>Signed-off-by: Guoshuai Li <ligs@dtdream.com>
> >>>---
> >>>
> >>>v3:
> >>>    Add garp-vlan test.
> >>>
> >>>v2:
> >>>    Add check vlan vaid.
> >>>    Add update localnet vlan tag process.
> >>Thanks a lot.  I applied this to master.  (I did change the type of
> >>'tag' from int64_t to just int.)
> >Also, if this is causing problems for older versions of OVN, let me know
> >and I'll cherry-pick it to an older branch.
> 
> hi, ben. I am sorry for the late reply, a few days ago on leave.
> 
> Since 2.6 branche, this problem exists.
> So I think it need cherry-pick to branch-2.8/branch-2.7/branch-2.6
> 
> Cherry-pick to branch-2.7/branch-2.6 is conflicted,  I sent a new commit:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/340854.html
> 

Thanks a lot.  I cherry-picked to branch-2.8 and then applied the
revised patch to branch-2.7 and branch-2.6.
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);
diff --git a/tests/ovn.at b/tests/ovn.at
index 6c38b973f..7ef32f5b1 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8426,6 +8426,37 @@  $PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/br-phys_n1-tx.pcap | trim_zeros
 AT_CHECK([grep $garp hv3_br_phys_tx | sort], [0], [expout])
 $PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv2/br-phys_n1-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq > hv2_br_phys_tx
 AT_CHECK([grep $garp hv2_br_phys_tx | sort], [0], [])
+
+# change localnet port tag.
+AT_CHECK([ovn-nbctl set Logical_Switch_Port ln_port tag=2014])
+
+# wait for earlier changes to take effect
+AT_CHECK([ovn-nbctl --timeout=3 --wait=sb sync], [0], [ignore])
+
+# update nat-addresses option
+ovn-nbctl --wait=sb lsp-set-options lrp0-rp router-port=lrp0
+ovn-nbctl --wait=sb lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router"
+
+as hv1 reset_pcap_file snoopvif hv1/snoopvif
+as hv2 reset_pcap_file br-phys_n1 hv2/br-phys_n1
+as hv3 reset_pcap_file br-phys_n1 hv3/br-phys_n1
+
+# Wait for packets to be received.
+OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100])
+trim_zeros() {
+    sed 's/\(00\)\{1,\}$//'
+}
+
+garp="fffffffffffff00000000001810007de08060001080006040001f00000000001c0a80064000000000000c0a80064"
+echo $garp > expout
+
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq >  hv1_snoopvif_tx
+AT_CHECK([sort hv1_snoopvif_tx], [0], [expout])
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv3/br-phys_n1-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq > hv3_br_phys_tx
+AT_CHECK([grep $garp hv3_br_phys_tx | sort], [0], [expout])
+$PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv2/br-phys_n1-tx.pcap | trim_zeros | only_broadcast_from_lrp1 | uniq > hv2_br_phys_tx
+AT_CHECK([grep $garp hv2_br_phys_tx | sort], [0], [])
+
 OVN_CLEANUP([hv1],[hv2],[hv3])
 
 AT_CLEANUP