ovsfw sometimes rejects legitimate traffic when multiple remote SG rules are in use

Bug #1708092 reported by IWAMOTO Toshihiro on 2017-08-02
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
High
IWAMOTO Toshihiro

Bug Description

ovsfw uses conjunction to represent SG rules with remote_group_id.
When there are multiple rules which differ only in remote_group_id, the ovsfw code generates flows with the same match fields and different conjuction actions. Such flows don't work well as the openflow spec says so.

An sequence to reproduce the bug:

$ openstack security group create sg1
$ openstack security group create sg2
$ openstack security group rule create --remote-group sg2 --dst-port 22:80 --protocol tcp --ingress sg1
$ openstack security group rule create --remote-group sg1 --dst-port 80 --protocol tcp --ingress sg1

Boot 3 instances: hoge1 (sg1), hoge2 (sg2), hoge12 (sg1 and sg2)

Start "nc -l -p 80" on hoge12.
Try to connect to hoge12:80 from hoge1 and hoge2. Either one should fail.

Jakub Libosvar (libosvar) wrote :

I was debugging today a failure where I saw flows with actions=conjunction() were not matched. When I replaced the conjunction action with output, it worked fine. Is it the same case?

IWAMOTO Toshihiro (iwamoto) wrote :

Hi Jakub,
that sounds like a different issue.

This bug report is about multiple flows with the same match fields but with different conjunction actions.

We need something like ConjIpManager that merges those actions=(...,2/2) flows.

Fix proposed to branch: master
Review: https://review.openstack.org/492404

Changed in neutron:
assignee: nobody → IWAMOTO Toshihiro (iwamoto)
status: New → In Progress
Changed in neutron:
importance: Undecided → High

Change abandoned by IWAMOTO Toshihiro (<email address hidden>) on branch: master
Review: https://review.openstack.org/489918
Reason: squashed into 492404.

Changed in neutron:
milestone: none → queens-2

Change abandoned by IWAMOTO Toshihiro (<email address hidden>) on branch: master
Review: https://review.openstack.org/512480

Reviewed: https://review.openstack.org/492404
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=237ec30ca94322716a1af5e59c0960f0eef24194
Submitter: Zuul
Branch: master

commit 237ec30ca94322716a1af5e59c0960f0eef24194
Author: IWAMOTO Toshihiro <email address hidden>
Date: Wed Aug 2 17:12:56 2017 +0900

    ovsfw: Merge multiple conjunction flows

    The ovsfw code generated multiple flows with the same or overlapping
    match fields and different actions=conjunction(nnn,2/2) flows.
    Merge such flows and generate only one flow with
    actions=conjunction(mmm,2/2),conjunction(nnn,2/2) so that filtering
    are correctly performed.

    Change-Id: I0cd325b02f35e103606595b8b124010fff8dc397
    Partial-bug: #1708092

Jakub Libosvar (libosvar) wrote :

Iwamoto, is there anything else we need to work on?

Reviewed: https://review.openstack.org/521786
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=04b155443a324d6d38b67e614085902963f315ec
Submitter: Zuul
Branch: stable/pike

commit 04b155443a324d6d38b67e614085902963f315ec
Author: IWAMOTO Toshihiro <email address hidden>
Date: Wed Aug 2 17:12:56 2017 +0900

    ovsfw: Merge multiple conjunction flows

    The ovsfw code generated multiple flows with the same or overlapping
    match fields and different actions=conjunction(nnn,2/2) flows.
    Merge such flows and generate only one flow with
    actions=conjunction(mmm,2/2),conjunction(nnn,2/2) so that filtering
    are correctly performed.

    Change-Id: I0cd325b02f35e103606595b8b124010fff8dc397
    Partial-bug: #1708092
    (cherry picked from commit 237ec30ca94322716a1af5e59c0960f0eef24194)

tags: added: in-stable-pike

Reviewed: https://review.openstack.org/494428
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=4ac4c22a646799aaecca61334a8bb44147ab881a
Submitter: Zuul
Branch: master

commit 4ac4c22a646799aaecca61334a8bb44147ab881a
Author: IWAMOTO Toshihiro <email address hidden>
Date: Thu Aug 17 15:13:53 2017 +0900

    ovsfw: Use multiple priorities in RULES_*_TABLE

    The OpenFlow spec says packets shouldn't match against multiple flows
    at the same priority or the result is undefined. In ovsfw, 8 priority
    levels are needed to comply with this rule.

    Note: unlike overlapping TCP port ranges cases, the current version
    of OVS seems to handle this case magically.

    Change-Id: I6deaee8dbe81453285b1fc685282952bc9456949
    Closes-bug: #1708092

Changed in neutron:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/524533
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=5c16f2bd79eabc7f7a069d9af9ff832dbdb4be54
Submitter: Zuul
Branch: stable/pike

commit 5c16f2bd79eabc7f7a069d9af9ff832dbdb4be54
Author: IWAMOTO Toshihiro <email address hidden>
Date: Thu Aug 17 15:13:53 2017 +0900

    ovsfw: Use multiple priorities in RULES_*_TABLE

    The OpenFlow spec says packets shouldn't match against multiple flows
    at the same priority or the result is undefined. In ovsfw, 8 priority
    levels are needed to comply with this rule.

    Note: unlike overlapping TCP port ranges cases, the current version
    of OVS seems to handle this case magically.

    Change-Id: I6deaee8dbe81453285b1fc685282952bc9456949
    Closes-bug: #1708092
    (cherry picked from commit 4ac4c22a646799aaecca61334a8bb44147ab881a)

This issue was fixed in the openstack/neutron 12.0.0.0b2 development milestone.

woody89 (a1007881221) wrote :

good

This issue was fixed in the openstack/neutron 11.0.3 release.

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.openstack.org/520251
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.opendev.org/520251
Reason: This review is > 4 weeks without comment, and failed Zuul jobs the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers