Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate several L4 OVN ACLs into smaller number of OVN ACLs #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jxiaobin
Copy link

@jxiaobin jxiaobin commented May 8, 2023

If an ingress rule has 5 TCP port specified along with a namespace
selector, then we end up creating 5 OVN ACLs. Instead, we could create
just one ACL by consoldiating the individual ports and/or ranges. For
example:

With this Network Policy,

spec:
ingress:
- from:
- nameSelector:
matchLabels:
name: web
ports:
- protocol: TCP
port: 3306
- protocol: TCP
port: 80
- protocol: TCP
port: 9000
endPort: 9100
we end up creating following 3 matches

(ip4.src == {$as1} && tcp && tcp.dst==3306 && outport == @pg1)
(ip4.src == {$as1} && tcp && tcp.dst==80 && outport == @pg1)
(ip4.src == {$as1} && tcp && 9000<=tcp.dst<=9100 && outport == @pg1)
With the fix in this commit, we will have only one match

(ip4.src == {$as1} && tcp && (tcp.dst=={3306,80} ||
9000<=tcp.dst<=9100 && outport == @pg1)
With reduced number of OVN ACLs will result in fewer logical flows
and few OpenFlow flows

Signed-off-by: Girish Moodalbail [email protected]

@jxiaobin jxiaobin force-pushed the us_acl_optimize branch 4 times, most recently from e4cfc4f to f1dfc82 Compare May 11, 2023 22:17
@jxiaobin jxiaobin requested a review from girishmg as a code owner July 5, 2023 07:11
@jxiaobin jxiaobin force-pushed the us_acl_optimize branch 7 times, most recently from 132f993 to d4c95f7 Compare July 10, 2023 16:46
@jxiaobin jxiaobin force-pushed the us_acl_optimize branch 5 times, most recently from 86d0471 to 05c1272 Compare July 27, 2023 22:24
If an ingress rule has 5 TCP port specified along with a namespace
selector, then we end up creating 5 OVN ACLs. Instead, we could create
just one ACL by consoldiating the individual ports and/or ranges. For
example:

With this Network Policy,

spec:
  ingress:
    - from:
      - nameSelector:
          matchLabels:
            name: web
      ports:
        - protocol: TCP
          port: 3306
        - protocol: TCP
          port: 80
        - protocol: TCP
          port: 9000
          endPort: 9100

we end up creating following 3 matches
(ip4.src == {$as1} && tcp && tcp.dst==3306 && outport == @pg1)
(ip4.src == {$as1} && tcp && tcp.dst==80 && outport == @pg1)
(ip4.src == {$as1} && tcp && 9000<=tcp.dst<=9100 && outport == @pg1)

With the fix in this commit, we will have only one match
(ip4.src == {$as1} && tcp && (tcp.dst=={3306,80} || \
  9000<=tcp.dst<=9100 && outport == @pg1)

With reduced number of OVN ACLs will result in fewer logical flows
and few OpenFlow flows

Signed-off-by: Xiaobin Qu <[email protected]>
Co-authored-by: Girish Moodalbail <[email protected]>
@girishmg girishmg force-pushed the master branch 3 times, most recently from 4d44be0 to 5044c18 Compare May 7, 2024 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant