Hi, Guru.
> + /* 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful table
> send
> + * packet to conntrack for defragmentation. */
+ const char *ip_address;
> + SSET_FOR_EACH(ip_address, &all_ips) {
> + char *match = xasprintf("ip && ip4.dst == %s", ip_address);
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
> + 100, match, REGBIT_CONNTRACK_DEFRAG" = 1;
> next;");
> + free(match);
> + }
>
For ls_in_pre_lb table, you add a same action from ls_in_pre_acl table.
Indeed, there will be lflows, such as:
table=3( ls_in_pre_acl), priority= 100, match=(ip), action=(reg0[0] = 1;
next;) [1]
table=4( ls_in_pre_lb), priority= 100, match=(ip && ip4.dst ==
30.0.0.1), action=(reg0[0] = 1; next;) [2]
I know it will be flexible to have different tables for different purpose,
but they are too close enough.
And for me, the only different is "&& ip4.dst == ENDPOINT_IP".
What's more, it's easily to have a allow-related ACL rules for traffic
whose initiater is VM/container. By that, in the most common case, we will
have rule [1] in table ls_in_pre_acl. If so, rule [2] in table ls_in_pre_lb
seems duplicated, and rules in table ls_out_pre_lb are duplicated to ones
in table ls_out_pre_acl.
So the only case to make pre_lb tables are necessary is, logical switch
doesn't contain an "allow-related" action ACL rule. It seems possible, but
I cannot figure out why people choose to not using "allow-related" action,
that will make ACL table hard to maintain.
+
> + sset_destroy(&all_ips);
> +
> + if (vip_configured) {
> + ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB,
> + 100, "ip", REGBIT_CONNTRACK_DEFRAG" = 1;
> next;");
> + }
> + }
> +}
> + if (od->nbs->load_balancer) {
> + struct nbrec_load_balancer *lb = od->nbs->load_balancer;
> + struct smap *vips = &lb->vips;
> + struct smap_node *node;
> +
> + SMAP_FOR_EACH (node, vips) {
> + uint16_t port = 0;
> +
> + /* node->key contains IP:port or just IP. */
> + char *ip_address = NULL;
> + ip_address_and_port_from_lb_key(node->key, &ip_address,
> &port);
> + if (!ip_address) {
> + continue;
> + }
> +
> + /* New connections in Ingress table. */
> + char *action = xasprintf("ct_lb(\"%s\");", node->value);
> + struct ds match = DS_EMPTY_INITIALIZER;
> + ds_put_format(&match, "ct.new && ip && ip4.dst == %s",
> ip_address);
> + if (port) {
> + if (lb->protocol && !strcmp(lb->protocol, "udp")) {
> + ds_put_format(&match, "&& udp && udp.dst == %d",
> port);
> + } else {
> + ds_put_format(&match, "&& tcp && tcp.dst == %d",
> port);
> + }
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_LB,
> + 120, ds_cstr(&match), action);
> + } else {
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_LB,
> + 110, ds_cstr(&match), action);
> + }
>
S_SWITCH_IN_LB, I think you missed to put them into method build_lb.
> +
> + ds_destroy(&match);
> + free(action);
> + }
> + }
> }
>
>
Thanks.
Zong Kai, LI
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
