| 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 |
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 >
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 --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);