Hi, Numan:
On Thursday, September 15, 2022 at 08:38:26 AM PDT, Numan Siddique
<[email protected]> wrote:
On Tue, Sep 13, 2022 at 6:41 PM venugopal iyer via dev
<[email protected]> wrote:
>
> Hi, Han, Numan:
> While testing a use case in our ovn-k8s cluster we ran into an issue where
> wecouldn't effectively use stateless ACL on the OVN interface. Turns out we
> will track
> all the packets here, since there will be a some VIPs configured on the
> logical
> switch and...
>
>
> if (od->has_lb_vip) {
> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
> 100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");...
>
> even though pre_acl skips stateless flows. One option is to just try an
> excludethe flows in pre_lb with hints from pre_acl, but that alone won't work
> since
> pre_lb precedes pre_acl in egress.
>
> So, wanted to check if the following will work: - Use info from pre_acl to
> exclude stateless flows when we track for vip- Switch pre_lb and pre_acl in
> egress.
>
> I tested this with our ovn-k8s cicd, manual stateless rules and using a
> specificuse case (RoCE with stateless rules) and it worked, but not sure if i
> am missinganything by switching pre acl and lb in egress.
>
> I can send the official patch, but just as an FYI, following is the change i
> amlooking at as a proof of concept:
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 98ef97f90..022b20660 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -128,8 +128,8 @@ enum ovn_stage {
> PIPELINE_STAGE(SWITCH, IN, L2_UNKNOWN, 25, "ls_in_l2_unknown") \
> \
> /* Logical switch egress stages. */ \
> - PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 0, "ls_out_pre_lb") \
> - PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 1, "ls_out_pre_acl") \
> + PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 0, "ls_out_pre_acl") \
> + PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 1, "ls_out_pre_lb") \
> PIPELINE_STAGE(SWITCH, OUT, PRE_STATEFUL, 2, "ls_out_pre_stateful") \
> PIPELINE_STAGE(SWITCH, OUT, ACL_HINT, 3, "ls_out_acl_hint") \
> PIPELINE_STAGE(SWITCH, OUT, ACL, 4, "ls_out_acl") \
> @@ -5839,6 +5839,54 @@ build_empty_lb_event_flow(struct ovn_lb_vip *lb_vip,
> return true;
> }
>
> static void
> build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
> {
> @@ -5904,12 +5952,17 @@ build_pre_lb(struct ovn_datapath *od, struct hmap
> *lflows)
> * To fix this issue, we send all the packets to the conntrack in the
> * ingress pipeline if a load balancer is configured. We can now
> * add a lflow to drop ct.inv packets.
> + *
> + * Skip stateless rules from being tracked.
> + * stateless flows should not have REGBIT_CONNTRACK_DEFRAG+ * set in
> build pre_acls, so we can use that as an hint here.
> */
> if (od->has_lb_vip) {
> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
> - 100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");
> + 100, REGBIT_CONNTRACK_DEFRAG" == 1",
> REGBIT_CONNTRACK_NAT" = 1; next;");
> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB,
> - 100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");
> + 100, REGBIT_CONNTRACK_DEFRAG" == 1",
> REGBIT_CONNTRACK_NAT" = 1; next;");
> }
> }
Hi Venu,
I'm not really sure if this would work. In the case of a logical
switch with no stateful ACLs and load balancers configured,
how would the traffic work if a pod sends a packet destined to the lb VIP ?
From what I understand with your proposed patch, the register bit
REGBIT_CONNTRACK_NAT will not be set now and hence the packet will not
be
sent to conntrack.
<vi> Thanks for your response, the above was just to quickly check if this
works; I'll work<vi> on the official patch to send for review, CI etc.
<vi> My main concern was the switch in the egress pipeline and wanted to see
if that<vi> has any fundamental concerns.
thanks,
-venu
Also I'd suggest posting your patch so that the CI gets run.
Thanks
Numan
>
>
>
>
> thanks,
> -venu
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
