Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

add --physdev-is-bridged flag when using physdev module for iptables #3453

Merged
merged 2 commits into from
Dec 5, 2018

Conversation

murali-reddy
Copy link
Contributor

add --physdev-is-bridged flag when using physdev module for iptables rules created by weave-npc to prevent warning in kernel logs

Fixes #3449

Since previous iptables chains rules are flushed on weave pod start, fix works for both fresh install and upgrades.

rules created by weave-npc to prevent warning in kernel logs

Fixes #3449
@murali-reddy
Copy link
Contributor Author

@bboreham I fixed (99e360c) below failing test (test/840_weave_kube_3_test.sh) with earlier commit.

# Check we can ping between the Weave bridge IPs on each host
HOST1EXPIP=$($SSH $HOST1 "weave expose" || true)
HOST2EXPIP=$($SSH $HOST2 "weave expose" || true)
HOST3EXPIP=$($SSH $HOST3 "weave expose" || true)
assert_raises "run_on $HOST1 $PING $HOST2EXPIP"
assert_raises "run_on $HOST2 $PING $HOST1EXPIP"
assert_raises "run_on $HOST3 $PING $HOST2EXPIP"

Using -m physdev --physdev-is-bridged fixes #3349, but it has side affect where a node can not access the weave bridge IP of any remote node. A node accessing remote node bridge Ip, does not get bridged but routed to host IP(as its local) hence missing below exception resulting in packet drop.

-A WEAVE-NPC-EGRESS -m physdev --physdev-in vethwe-bridge --physdev-is-bridged -j RETURN
Not sure if these tests are required. At least from Kubernetes networking requirements point of view we don't need to care about the ip assigned to bridge IP. Is connectivity between weave bridge IP's needed across the nodes?

@bboreham
Copy link
Contributor

bboreham commented Dec 4, 2018

The weave bridge ip is used for “service import” which has been a feature since nearly the beginning.

I think it would be ok to say “you can’t have service import and network policy at the same time”.

That said, I don’t quite understand your statement of the problem. The sending node doesn’t know what the destination IP is, does it? Is the problem on the receiving node?

Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@murali-reddy
Copy link
Contributor Author

I have finished sanity testing for various scenarios for any possible regression due to addition of new rule. its good to be get merged.

-A WEAVE-NPC-EGRESS -m addrtype --dst-type LOCAL -j RETURN

@bboreham bboreham merged commit a332af3 into 2.5 Dec 5, 2018
@bboreham bboreham deleted the issue-3449 branch December 24, 2018 12:03
@bboreham bboreham added this to the 2.5.1 milestone Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants