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

"Duplicate" flows installed in IngressRule table for local Node traffic #191

Closed
antoninbas opened this issue Dec 6, 2019 · 1 comment
Closed
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@antoninbas
Copy link
Contributor

Describe the bug
Not really a bug per se, but probably involuntary and a bit confusing when looking at the flow table. This is what I get when I dump flows for IngressRule on my testbed (after installing a simple network policy):

1.  table=90, n_packets=0, n_bytes=0, priority=210,ct_state=-new+est,ip actions=resubmit(,110)
2.  table=90, n_packets=0, n_bytes=0, priority=210,ip,nw_src=10.10.1.1 actions=resubmit(,110)
3.  table=90, n_packets=0, n_bytes=0, priority=200,ip,nw_src=10.10.1.1 actions=conjunction(1,1/3)
4.  table=90, n_packets=0, n_bytes=0, priority=200,ip,nw_src=10.10.1.2 actions=conjunction(1,1/3)
5.  table=90, n_packets=0, n_bytes=0, priority=200,ip,nw_src=10.10.1.3 actions=conjunction(1,1/3)
6.  table=90, n_packets=0, n_bytes=0, priority=200,ip,reg1=0x3 actions=conjunction(1,2/3)
7.  table=90, n_packets=0, n_bytes=0, priority=200,ip,reg1=0x4 actions=conjunction(1,2/3)
8.  table=90, n_packets=0, n_bytes=0, priority=200,tcp,tp_dst=80 actions=conjunction(1,3/3)
9.  table=90, n_packets=0, n_bytes=0, priority=190,conj_id=1,ip actions=resubmit(,110)
10. table=90, n_packets=0, n_bytes=0, priority=80,ip actions=resubmit(,100)

It seems to me that flow 3 is not required because we already have flow 2, which has higher priority and is more "generic".

To Reproduce
Install a simple network policy like this one:

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: test-network-policy
  namespace: default
spec:
  podSelector:
    matchLabels:
      app: nginx
  policyTypes:
  - Ingress
  - Egress
  ingress:
  - from:
    - podSelector:
        matchLabels:
          app: nginx
    ports:
    - protocol: TCP
      port: 80
  egress:
  - to:
    - podSelector:
        matchLabels:
          app: nginx
    ports:
    - protocol: TCP
      port: 80

then start some Pods with label app=nginx and dump the flows on the Node on which the Pods are scheduled.

Versions:
Antrea:

===> Version information <===
VERSION: v0.2.0-dev
GIT_SHA: 86ccebe
GIT_TREE_STATE: clean
RELEASE_STATUS: unreleased
DOCKER_IMG_VERSION: v0.2.0-dev-86ccebe

Additional context
We had 2 patches merged which seem to have a similar goal:

Maybe we should just revert #104. @tnqn am I missing something?

@antoninbas
Copy link
Contributor Author

@tnqn I just noticed your comment: #176 (comment)
I'll open the PR to revert.

@McCodeman McCodeman added the kind/bug Categorizes issue or PR as related to a bug. label Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants