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

Already on GitHub? Sign in to your account

Removing duplicated 'drop' entries in scripts/setup-vif-rules #940

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

amarao commented Dec 9, 2012

reason: they are always before global 'drop everything else'.
Comments about specific icmp message types are saved as important info.

George Shuklin Removing excessive icmp ipv6 drops from setup-vif-rules
reason: they are always before global 'drop everything else'.
Comments about specific icmp message types are saved as important info.
d19a9fc
Owner

xen-git commented Dec 9, 2012

Can one of the admins verify this patch?

@ghost ghost assigned robhoes Dec 9, 2012

Owner

jonludlam commented Dec 9, 2012

@robhoes could you take a look, please?

Owner

robhoes commented Dec 12, 2012

The order in which the add_flow function is called by that bit of code is misleading. The actual flow table is sorted by priority. The rules you are removing have priority 6000, which is higher than some IPv6 rules with action=normal, a little further up in the code. Therefore, I think that removing those rules will lead to certain packets going through rather than getting dropped.

Collaborator

djs55 commented Dec 12, 2012

Should we re-order the code to match the priorities or is that too tricky?

On Wednesday, December 12, 2012, Rob Hoes wrote:

The order in which the add_flow function is called by that bit of code is
misleading. The actual flow table is sorted by priority. The rules you are
removing have priority 6000, which is higher than some IPv6 rules with
action=normal, a little further up in the code. Therefore, I think that
removing those rules will lead to certain packets going through rather than
getting dropped.


Reply to this email directly or view it on GitHubhttps://github.com/xen-org/xen-api/pull/940#issuecomment-11294588.

Dave Scott

Owner

robhoes commented Dec 13, 2012

@djs55 Some of the rules are generated in loops, and we won't be able to put the rules in priority order, unless we split the loops. It's possible, but I don't think it will make things much clearer...

Contributor

amarao commented Dec 13, 2012

Oops,, I was wrong, sorry. I missed generic 'allow icmp v6' line:

add_flow(bridge_name,
"in_port=%s,priority=5000,dl_src=%s,ipv6_src=%s,icmp6,action=normal" %
(port, mac, ipv6))

PS Those rules still disallow any generic ipv6 traffic except some
ICMPs, TCP6, UDP6. How about other IPv6-based protocols?

13.12.2012 15:18, Rob Hoes пишет:

@djs55 https://github.com/djs55 Some of the rules are generated in
loops, and we won't be able to put the rules in priority order, unless
we split the loops. It's possible, but I don't think it will make
things much clearer...


Reply to this email directly or view it on GitHub
xen-org#940 (comment).

Owner

jonludlam commented Dec 18, 2012

I'm going to close this pull request for now - please resubmit if you feel there's still an issue here to fix. Thanks!

@jonludlam jonludlam closed this Dec 18, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment