| Message ID | 20220519201733.2184302-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] tests: Fix failing test case - "Port security lflows" | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Thanks for the quick fix on this Numan. I went ahead and pushed it to main. On 5/19/22 16:17, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > Also added a few comments in the port security related functions > in controller/lflow.c > > Fixes: 94974a02bfd5("northd: Add generic port security logical flows.") > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > controller/lflow.c | 16 +++++++++++++++- > tests/ovn-northd.at | 2 +- > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/controller/lflow.c b/controller/lflow.c > index e968231492..7a3419305a 100644 > --- a/controller/lflow.c > +++ b/controller/lflow.c > @@ -2731,7 +2731,13 @@ lflow_handle_flows_for_lport(const struct sbrec_port_binding *pb, > return false; > } > > - /* Program the port security flows. */ > + /* Program the port security flows. > + * Note: All the port security OF rules are added using the 'uuid' > + * of the port binding. Right now port binding 'uuid' is used in > + * the logical flow table (l_ctx_out->flow_table) only for port > + * security flows. Later if new flows are added using the > + * port binding'uuid', then this function should handle it properly. > + */ > ofctrl_remove_flows(l_ctx_out->flow_table, &pb->header_.uuid); > > if (pb->n_port_security && shash_find(l_ctx_in->binding_lports, > @@ -3399,6 +3405,10 @@ build_out_port_sec_ip4_flows(const struct sbrec_port_binding *pb, > struct ovn_desired_flow_table *flow_table) > { > if (!ps_addr->n_ipv4_addrs && !ps_addr->n_ipv6_addrs) { > + /* No IPv4 and no IPv6 addresses in the port security. > + * Both IPv4 and IPv6 traffic should be delivered to the > + * lport. build_out_port_sec_no_ip_flows() takes care of > + * adding the required flow(s) to allow. */ > return; > } > > @@ -3493,6 +3503,10 @@ build_out_port_sec_ip6_flows(const struct sbrec_port_binding *pb, > struct ovn_desired_flow_table *flow_table) > { > if (!ps_addr->n_ipv4_addrs && !ps_addr->n_ipv6_addrs) { > + /* No IPv4 and no IPv6 addresses in the port security. > + * Both IPv4 and IPv6 traffic should be delivered to the > + * lport. build_out_port_sec_no_ip_flows() takes care of > + * adding the required flow(s) to allow. */ > return; > } > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 0912e3bec4..5bd0935e72 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -7404,8 +7404,8 @@ AT_CHECK([cat sw0flows | grep -e port_sec | sort | sed 's/table=./table=?/' ], [ > table=? (ls_in_check_port_sec), priority=100 , match=(eth.src[[40]]), action=(drop;) > table=? (ls_in_check_port_sec), priority=100 , match=(vlan.present), action=(drop;) > table=? (ls_in_check_port_sec), priority=50 , match=(1), action=(reg0[[15]] = check_in_port_sec(); next;) > - table=? (ls_in_check_port_sec), priority=50 , match=(inport == "sw0p1"), action=(reg0[[14]] = 1; next(pipeline=ingress, table=16);) > table=? (ls_in_check_port_sec), priority=70 , match=(inport == "localnetport"), action=(set_queue(10); reg0[[15]] = check_in_port_sec(); next;) > + table=? (ls_in_check_port_sec), priority=70 , match=(inport == "sw0p1"), action=(reg0[[14]] = 1; next(pipeline=ingress, table=16);) > table=? (ls_in_check_port_sec), priority=70 , match=(inport == "sw0p2"), action=(set_queue(10); reg0[[15]] = check_in_port_sec(); next;) > table=? (ls_in_apply_port_sec), priority=0 , match=(1), action=(next;) > table=? (ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1), action=(drop;) >
diff --git a/controller/lflow.c b/controller/lflow.c index e968231492..7a3419305a 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -2731,7 +2731,13 @@ lflow_handle_flows_for_lport(const struct sbrec_port_binding *pb, return false; } - /* Program the port security flows. */ + /* Program the port security flows. + * Note: All the port security OF rules are added using the 'uuid' + * of the port binding. Right now port binding 'uuid' is used in + * the logical flow table (l_ctx_out->flow_table) only for port + * security flows. Later if new flows are added using the + * port binding'uuid', then this function should handle it properly. + */ ofctrl_remove_flows(l_ctx_out->flow_table, &pb->header_.uuid); if (pb->n_port_security && shash_find(l_ctx_in->binding_lports, @@ -3399,6 +3405,10 @@ build_out_port_sec_ip4_flows(const struct sbrec_port_binding *pb, struct ovn_desired_flow_table *flow_table) { if (!ps_addr->n_ipv4_addrs && !ps_addr->n_ipv6_addrs) { + /* No IPv4 and no IPv6 addresses in the port security. + * Both IPv4 and IPv6 traffic should be delivered to the + * lport. build_out_port_sec_no_ip_flows() takes care of + * adding the required flow(s) to allow. */ return; } @@ -3493,6 +3503,10 @@ build_out_port_sec_ip6_flows(const struct sbrec_port_binding *pb, struct ovn_desired_flow_table *flow_table) { if (!ps_addr->n_ipv4_addrs && !ps_addr->n_ipv6_addrs) { + /* No IPv4 and no IPv6 addresses in the port security. + * Both IPv4 and IPv6 traffic should be delivered to the + * lport. build_out_port_sec_no_ip_flows() takes care of + * adding the required flow(s) to allow. */ return; } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 0912e3bec4..5bd0935e72 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -7404,8 +7404,8 @@ AT_CHECK([cat sw0flows | grep -e port_sec | sort | sed 's/table=./table=?/' ], [ table=? (ls_in_check_port_sec), priority=100 , match=(eth.src[[40]]), action=(drop;) table=? (ls_in_check_port_sec), priority=100 , match=(vlan.present), action=(drop;) table=? (ls_in_check_port_sec), priority=50 , match=(1), action=(reg0[[15]] = check_in_port_sec(); next;) - table=? (ls_in_check_port_sec), priority=50 , match=(inport == "sw0p1"), action=(reg0[[14]] = 1; next(pipeline=ingress, table=16);) table=? (ls_in_check_port_sec), priority=70 , match=(inport == "localnetport"), action=(set_queue(10); reg0[[15]] = check_in_port_sec(); next;) + table=? (ls_in_check_port_sec), priority=70 , match=(inport == "sw0p1"), action=(reg0[[14]] = 1; next(pipeline=ingress, table=16);) table=? (ls_in_check_port_sec), priority=70 , match=(inport == "sw0p2"), action=(set_queue(10); reg0[[15]] = check_in_port_sec(); next;) table=? (ls_in_apply_port_sec), priority=0 , match=(1), action=(next;) table=? (ls_in_apply_port_sec), priority=50 , match=(reg0[[15]] == 1), action=(drop;)