kytos-ng / flow_manager

Kytos NApp that manages OpenFlow 1.3 entries
https://kytos-ng.github.io/api/flow_manager.html
MIT License
0 stars 7 forks source link

Incorrect matching for flow deletion #196

Open Alopalao opened 2 months ago

Alopalao commented 2 months ago

Kytos should maybe start matching by "owner" as well. For example with RingTopo and this pipeline enabled:

{
    "multi_table": [
        {
            "table_id": 1,
            "description": "Second table for coloring",
            "napps_table_groups": {
                "coloring": ["base"]
            },
            "table_miss_flow": {
                "priority": 0,
                "instructions": [{
                    "instruction_type": "goto_table",
                    "table_id": 2
                }]
            }
        }
    ]
}

Then we disable interface 00:03:3. The flow from coloring will delete of_multi_table flow because the miss flows do not have match field. These flows are going to be matched. coloring -> {'table_id': 1, 'match': {'dl_src': 'ee:ee:ee:ee:ee:01'}} of_multi_table -> {'table_id': 1, 'owner': 'of_multi_table', 'table_group': 'base', 'priority': 0, 'cookie': 12465963768561532929, 'idle_timeout': 0, 'hard_timeout': 0, 'instructions': [{'instruction_type': 'goto_table', 'table_id': 2}]}

viniarck commented 2 months ago

@Alopalao, good catch.

The non strict match deletion that we have on flow_manager implements an OpenFlow match, so the owner shouldn't be included there, although it's part of our flow data model NApp ownership as you know.

I think the issue is on this line here https://github.com/kytos-ng/flow_manager/blob/master/v0x04/match.py#L46, or "match" not in stored_flow_dict would match even if the flow being installed has a match. The general all deletion that would match anything would be only if what's being deleted doesn't have a match defined.

I think we can nuke the latter part of that logical or. We need to confirm:

This bug is important, I'll leave with priority_medium. It hasn't been reported in prod yet due to network engineers not using yet the flow miss entry or any other similar flow.