| Message ID | 32746328541e298b339620e73867583b14d4652a.1599148681.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] northd: fix empty_lb_backends controller_event for IPv6 | expand |
On 9/3/20 6:04 PM, Lorenzo Bianconi wrote: > Introduce missing square brackets defining IPv6 empty_lb_backends > controller_event logical flows in order to properly extract vip ip > and port from the related string > > Fixes: bb9f2b9ce56c ("ovn-northd: Consider load balancer active backends in router pipeline") > Fixes: 821e1e54abcb ("OVN: use trigger_event action to report 'empty_lb_rule' events") > Suggested-by: Dumitru Ceara <dceara@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > northd/ovn-northd.c | 7 ++++--- > tests/ovn.at | 30 +++++++++++++++++++++++++++--- > 2 files changed, 31 insertions(+), 6 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 7be0e85e6..f05e8ae6f 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -4996,6 +4996,7 @@ build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows, > return; > } > > + bool ipv4 = lb_vip->addr_family == AF_INET; Hi Lorenzo, Nit: I would add parenthesis here: bool ipv4 = (lb_vip->addr_family == AF_INET); Otherwise, the patch looks good to me so with this addressed: Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks, Dumitru > struct ds match = DS_EMPTY_INITIALIZER; > char *meter = "", *action; > > @@ -5004,14 +5005,14 @@ build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows, > } > > ds_put_format(&match, "ip%s.dst == %s && %s", > - lb_vip->addr_family == AF_INET ? "4": "6", > - lb_vip->vip, lb->protocol); > + ipv4 ? "4": "6", lb_vip->vip, lb->protocol); > > char *vip = lb_vip->vip; > if (lb_vip->vip_port) { > ds_put_format(&match, " && %s.dst == %u", lb->protocol, > lb_vip->vip_port); > - vip = xasprintf("%s:%u", lb_vip->vip, lb_vip->vip_port); > + vip = xasprintf("%s%s%s:%u", ipv4 ? "" : "[", lb_vip->vip, > + ipv4 ? "" : "]", lb_vip->vip_port); > } > > action = xasprintf("trigger_event(event = \"%s\", " > diff --git a/tests/ovn.at b/tests/ovn.at > index 5ad51c095..f659cdfa8 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -16411,7 +16411,7 @@ ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 > for i in 0 1; do > idx=$((i+1)) > ovn-nbctl ls-add sw$i > - ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24 > + ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24 200$idx::a/64 > ovn-nbctl \ > -- lsp-add sw$i lrp$i-attachment \ > -- set Logical_Switch_Port lrp$i-attachment type=router \ > @@ -16427,7 +16427,7 @@ for i in 1 2; do > > for j in 1 2; do > ovn-nbctl lsp-add sw0 sw0-p$i$j -- \ > - lsp-set-addresses sw0-p$i$j "00:00:00:00:00:$i$j 192.168.1.$i$j" > + lsp-set-addresses sw0-p$i$j "00:00:00:00:00:$i$j 192.168.1.$i$j 2001::$i$j" > > ovs-vsctl -- add-port br-int vif$i$j -- \ > set interface vif$i$j \ > @@ -16440,7 +16440,7 @@ done > > as hv1 > ovn-nbctl lsp-add sw1 sw1-p0 \ > - -- lsp-set-addresses sw1-p0 "00:00:00:00:00:33 192.168.2.11" > + -- lsp-set-addresses sw1-p0 "00:00:00:00:00:33 192.168.2.11 2002::1" > ovs-vsctl -- add-port br-int vif33 -- \ > set interface vif33 \ > external-ids:iface-id=sw1-p0 \ > @@ -16456,6 +16456,11 @@ uuid_lb0=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0) > ovn-nbctl lb-add lb1 192.168.2.100:80 "" > ovn-nbctl lr-lb-add lr0 lb1 > uuid_lb1=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb1) > + > +ovn-nbctl lb-add lb2 [[2001::10]]:50051 "" > +ovn-nbctl ls-lb-add sw0 lb2 > +uuid_lb2=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb2) > + > ovn-nbctl --wait=hv meter-add event-elb drop 100 pktps 10 > > OVN_POPULATE_ARP > @@ -16500,6 +16505,25 @@ empty_lb_backends > AT_CHECK([ovn-sbctl get controller_event $uuid event_info:vip], [0], [dnl > "192.168.2.100:80" > ]) > +ovn-sbctl destroy controller_event $uuid > + > +packet2="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 && eth.dst==00:00:00:00:00:21 && > + ip6 && ip.ttl==64 && ip6.src==2001::11 && ip6.dst==2001::10 && > + tcp && tcp.src==10000 && tcp.dst==50051" > + > +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet2" > +ovn-sbctl list controller_event > +uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}') > + > +AT_CHECK([ovn-sbctl get controller_event $uuid event_type], [0], [dnl > +empty_lb_backends > +]) > +AT_CHECK([ovn-sbctl get controller_event $uuid event_info:vip], [0], [dnl > +"[[2001::10]]:50051" > +]) > +AT_CHECK_UNQUOTED([ovn-sbctl get controller_event $uuid event_info:load_balancer], [0], [dnl > +"$uuid_lb2" > +]) > > OVN_CLEANUP([hv1], [hv2]) > AT_CLEANUP >
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 7be0e85e6..f05e8ae6f 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -4996,6 +4996,7 @@ build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows, return; } + bool ipv4 = lb_vip->addr_family == AF_INET; struct ds match = DS_EMPTY_INITIALIZER; char *meter = "", *action; @@ -5004,14 +5005,14 @@ build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows, } ds_put_format(&match, "ip%s.dst == %s && %s", - lb_vip->addr_family == AF_INET ? "4": "6", - lb_vip->vip, lb->protocol); + ipv4 ? "4": "6", lb_vip->vip, lb->protocol); char *vip = lb_vip->vip; if (lb_vip->vip_port) { ds_put_format(&match, " && %s.dst == %u", lb->protocol, lb_vip->vip_port); - vip = xasprintf("%s:%u", lb_vip->vip, lb_vip->vip_port); + vip = xasprintf("%s%s%s:%u", ipv4 ? "" : "[", lb_vip->vip, + ipv4 ? "" : "]", lb_vip->vip_port); } action = xasprintf("trigger_event(event = \"%s\", " diff --git a/tests/ovn.at b/tests/ovn.at index 5ad51c095..f659cdfa8 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -16411,7 +16411,7 @@ ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1 for i in 0 1; do idx=$((i+1)) ovn-nbctl ls-add sw$i - ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24 + ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24 200$idx::a/64 ovn-nbctl \ -- lsp-add sw$i lrp$i-attachment \ -- set Logical_Switch_Port lrp$i-attachment type=router \ @@ -16427,7 +16427,7 @@ for i in 1 2; do for j in 1 2; do ovn-nbctl lsp-add sw0 sw0-p$i$j -- \ - lsp-set-addresses sw0-p$i$j "00:00:00:00:00:$i$j 192.168.1.$i$j" + lsp-set-addresses sw0-p$i$j "00:00:00:00:00:$i$j 192.168.1.$i$j 2001::$i$j" ovs-vsctl -- add-port br-int vif$i$j -- \ set interface vif$i$j \ @@ -16440,7 +16440,7 @@ done as hv1 ovn-nbctl lsp-add sw1 sw1-p0 \ - -- lsp-set-addresses sw1-p0 "00:00:00:00:00:33 192.168.2.11" + -- lsp-set-addresses sw1-p0 "00:00:00:00:00:33 192.168.2.11 2002::1" ovs-vsctl -- add-port br-int vif33 -- \ set interface vif33 \ external-ids:iface-id=sw1-p0 \ @@ -16456,6 +16456,11 @@ uuid_lb0=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0) ovn-nbctl lb-add lb1 192.168.2.100:80 "" ovn-nbctl lr-lb-add lr0 lb1 uuid_lb1=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb1) + +ovn-nbctl lb-add lb2 [[2001::10]]:50051 "" +ovn-nbctl ls-lb-add sw0 lb2 +uuid_lb2=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb2) + ovn-nbctl --wait=hv meter-add event-elb drop 100 pktps 10 OVN_POPULATE_ARP @@ -16500,6 +16505,25 @@ empty_lb_backends AT_CHECK([ovn-sbctl get controller_event $uuid event_info:vip], [0], [dnl "192.168.2.100:80" ]) +ovn-sbctl destroy controller_event $uuid + +packet2="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 && eth.dst==00:00:00:00:00:21 && + ip6 && ip.ttl==64 && ip6.src==2001::11 && ip6.dst==2001::10 && + tcp && tcp.src==10000 && tcp.dst==50051" + +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet2" +ovn-sbctl list controller_event +uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}') + +AT_CHECK([ovn-sbctl get controller_event $uuid event_type], [0], [dnl +empty_lb_backends +]) +AT_CHECK([ovn-sbctl get controller_event $uuid event_info:vip], [0], [dnl +"[[2001::10]]:50051" +]) +AT_CHECK_UNQUOTED([ovn-sbctl get controller_event $uuid event_info:load_balancer], [0], [dnl +"$uuid_lb2" +]) OVN_CLEANUP([hv1], [hv2]) AT_CLEANUP
Introduce missing square brackets defining IPv6 empty_lb_backends controller_event logical flows in order to properly extract vip ip and port from the related string Fixes: bb9f2b9ce56c ("ovn-northd: Consider load balancer active backends in router pipeline") Fixes: 821e1e54abcb ("OVN: use trigger_event action to report 'empty_lb_rule' events") Suggested-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- northd/ovn-northd.c | 7 ++++--- tests/ovn.at | 30 +++++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-)