openvswitch / ovs-issues

Issue tracker repo for Open vSwitch
10 stars 3 forks source link

Issues on encoding/decoding instructions during pause-resume process #213

Closed GraysonWu closed 2 years ago

GraysonWu commented 3 years ago

My env

Open vSwitch Version 2.14.0
OpenFlow Version 1.3

My goal I want to use the controller action with the pause flag to pause the pipeline. Later, the controller sends the continuation back to the OVS to resume the pipeline.

What I have done I used the NXAST_RAW_CONTROLLER2 message within the flow mod message to add a flow with controller action with the pause flag on the OVS. This part works well, as I can see it is added successfully:

$ ovs-ofctl dump-flows br-int table=85 -O OpenFlow13
..., table=85, n_packets=0,..., actions=controller(reason=no_match,id=65127,userdata=0c.00.00.00,pause),goto_table:105

And then I sent an NXT_SET_PACKET_IN_FORMAT message from the controller to the OVS to set the packet_in format as OFPUTIL_PACKET_IN_NXT2. After these setups, I created a traffic which hit the flow above. The controller could receive the packet_in2 message successfully. But when the controller sent a NXT_RESUME message back, I met the issue.

Issue I met After I send NXT_RESUME message to the OVS, I received OpenFlow1.3 error: OFPBAC_BAD_ARGUMENT. The OVS log:

2021-06-17T23:55:32.025Z|00091|ofp_actions|INFO|OFPAT_SET_VLAN_VID is deprecated in OpenFlow13 (use Set-Field)
2021-06-17T23:55:32.025Z|00092|ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_ARGUMENT):
00000000  00 01 00 08 69 00 00 00-
2021-06-17T23:55:32.025Z|00093|connmgr|INFO|br-int<->unix#0: sending OFPBAC_BAD_ARGUMENT error reply to NXT_RESUME message

But since I didn't change anything in the NXPINT_CONINUATION that I received, why the action inside the NXPINT_CONINUATION.NXCPT_ACTIONS is invalid?

I checked the packet_in2 that the controller received from the OVS:

00000000  04 04 01 48 00 00 00 00  00 00 23 20 00 00 00 1e  |...H......# ....|
00000010  00 00 00 66 5a d7 27 34  a3 fe 56 5d ca 5b 34 fc  |...fZ.'4..V].[4.|
00000020  08 00 45 00 00 54 9c 43  40 00 40 01 88 4a 0a 0a  |..E..T.C@.@..J..|
00000030  01 03 0a 0a 01 05 08 00  77 ae 12 e0 00 04 f7 e0  |........w.......|
00000040  cb 60 00 00 00 00 ea 58  01 00 00 00 00 00 10 11  |.`.....X........|
00000050  12 13 14 15 16 17 18 19  1a 1b 1c 1d 1e 1f 20 21  |.............. !|
00000060  22 23 24 25 26 27 28 29  2a 2b 2c 2d 2e 2f 30 31  |"#$%&'()*+,-./01|
00000070  32 33 34 35 36 37 00 00  00 03 00 05 00 00 00 00  |234567..........|
00000080  00 04 00 10 00 00 00 00  00 9b 00 00 00 00 00 00  |................|
00000090  00 05 00 05 00 00 00 00  00 06 00 65 80 00 00 04  |...........e....|
000000a0  00 00 00 04 80 00 0a 02  08 00 00 01 00 04 03 51  |...............Q|
000000b0  00 02 00 01 02 04 00 00  00 06 00 01 06 04 00 00  |................|
000000c0  00 01 00 01 08 04 00 01  00 00 00 01 d3 08 00 00  |................|
000000d0  00 21 00 00 00 ff 00 01  d4 02 ff f0 00 01 f0 04  |.!..............|
000000e0  0a 0a 01 03 00 01 f2 04  0a 0a 01 05 00 01 ee 01  |................|
000000f0  01 00 01 f8 02 00 08 00  01 fa 02 00 00 00 00 00  |................|
00000100  00 08 00 40 00 00 00 00  80 00 00 14 36 d5 68 3d  |...@........6.h=|
00000110  87 42 dd 11 a3 a8 f2 88  0a ae 84 b3 00 00 00 00  |.B..............|
00000120  80 03 00 04 00 00 00 00  80 06 00 10 00 00 00 00  |................|
00000130  00 01 00 08 69 00 00 00  80 08 00 08 00 00 00 04  |....i...........|
00000140  00 07 00 08 0c 00 00 00                           |........|

Based on my understanding, the NXPINT_CONINUATION part inside that packet_in2 message above are these bytes:

00000100  00 08 00 40 00 00 00 00  80 00 00 14 36 d5 68 3d  |...@........6.h=|
00000110  87 42 dd 11 a3 a8 f2 88  0a ae 84 b3 00 00 00 00  |.B..............|
00000120  80 03 00 04 00 00 00 00  80 06 00 10 00 00 00 00  |................|
00000130  00 01 00 08 69 00 00 00  80 08 00 08 00 00 00 04  |....i...........|

Details of these bytes:

OVS encodes actions into bytes

Seems like that while the OVS sending the packet_in2 to the controller, the OVS encodes the goto_table:105, the action after pause in my flow above, into the NXCPT_ACTIONS. The structure of goto_table and the type of instruction are: https://github.com/openvswitch/ovs/blob/master/include/openflow/openflow-1.1.h#L249 https://github.com/openvswitch/ovs/blob/master/include/openflow/openflow-1.1.h#L273

/* Instruction structure for OFPIT_GOTO_TABLE */
struct ofp11_instruction_goto_table {
    ovs_be16 type;                 /* OFPIT_GOTO_TABLE */
    ovs_be16 len;                  /* Length of this struct in bytes. */
    uint8_t table_id;              /* Set next table in the lookup pipeline */
    uint8_t pad[3];                /* Pad to 64 bits. */
};

enum ofp11_instruction_type {
    OFPIT11_GOTO_TABLE = 1,        /* Setup the next table in the lookup
                                      pipeline */
    OFPIT11_WRITE_METADATA = 2,    /* Setup the metadata field for use later
                                      in pipeline */
    OFPIT11_WRITE_ACTIONS = 3,     /* Write the action(s) onto the datapath
                                      action set */
    OFPIT11_APPLY_ACTIONS = 4,     /* Applies the action(s) immediately */
    OFPIT11_CLEAR_ACTIONS = 5,     /* Clears all actions from the datapath
                                      action set */
    OFPIT11_EXPERIMENTER = 0xFFFF  /* Experimenter instruction */
};

Thus, if we encode goto_table:105 into bytes, it will be 00 01 00 08 69 00 00 00 (type: 0x0001, len: 0x0008, table_id: 0x0069=105, pad[3]) which is exactly the same as what I received.

OVS decodes bytes into actions

When the controller send the NXPINT_CONINUATION back to the OVS, OVS should decode 00 01 00 08 69 00 00 00 to action. But while decoding the 00 01, seems that the OVS treat it as OFPACT_SET_VLAN_VID instead of an instruction goto_table. The structure of ofpact_vlan_vid is: https://github.com/openvswitch/ovs/blob/master/include/openvswitch/ofp-actions.h#L414

/* OFPACT_SET_VLAN_VID.
 *
 * We keep track if vlan was present at action validation time to avoid a
 * PUSH_VLAN when translating to OpenFlow 1.1+.
 *
 * We also keep the originating OFPUTIL action code in ofpact.compat.
 *
 * Used for OFPAT10_SET_VLAN_VID and OFPAT11_SET_VLAN_VID. */
struct ofpact_vlan_vid {
    OFPACT_PADDED_MEMBERS(
        struct ofpact ofpact;
        uint16_t vlan_vid;        /* VLAN VID in low 12 bits, other bits 0. */
        bool push_vlan_if_needed; /* OF 1.0 semantics if true. */
        bool flow_has_vlan;       /* VLAN present at action validation time? */
    );
};

Then 69 00 will be treated as vlan_vid. An error will be throughout from here https://github.com/openvswitch/ovs/blob/master/lib/ofp-actions.c#L1566.

The resume message I sent to the OVS is:

00000000  04 04 01 40 00 00 00 00  00 00 23 20 00 00 00 1c  |...H......# ....|
00000010  00 00 00 66 5a d7 27 34  a3 fe 56 5d ca 5b 34 fc  |...fZ.'4..V].[4.|
00000020  08 00 45 00 00 54 9c 43  40 00 40 01 88 4a 0a 0a  |..E..T.C@.@..J..|
00000030  01 03 0a 0a 01 05 08 00  77 ae 12 e0 00 04 f7 e0  |........w.......|
00000040  cb 60 00 00 00 00 ea 58  01 00 00 00 00 00 10 11  |.`.....X........|
00000050  12 13 14 15 16 17 18 19  1a 1b 1c 1d 1e 1f 20 21  |.............. !|
00000060  22 23 24 25 26 27 28 29  2a 2b 2c 2d 2e 2f 30 31  |"#$%&'()*+,-./01|
00000070  32 33 34 35 36 37 00 00  00 03 00 05 00 00 00 00  |234567..........|
00000080  00 04 00 10 00 00 00 00  00 9b 00 00 00 00 00 00  |................|
00000090  00 05 00 05 00 00 00 00  00 06 00 65 80 00 00 04  |...........e....|
000000a0  00 00 00 04 80 00 0a 02  08 00 00 01 00 04 03 51  |...............Q|
000000b0  00 02 00 01 02 04 00 00  00 06 00 01 06 04 00 00  |................|
000000c0  00 01 00 01 08 04 00 01  00 00 00 01 d3 08 00 00  |................|
000000d0  00 21 00 00 00 ff 00 01  d4 02 ff f0 00 01 f0 04  |.!..............|
000000e0  0a 0a 01 03 00 01 f2 04  0a 0a 01 05 00 01 ee 01  |................|
000000f0  01 00 01 f8 02 00 08 00  01 fa 02 00 00 00 00 00  |................|
00000100  00 08 00 40 00 00 00 00  80 00 00 14 36 d5 68 3d  |...@........6.h=|
00000110  87 42 dd 11 a3 a8 f2 88  0a ae 84 b3 00 00 00 00  |.B..............|
00000120  80 03 00 04 00 00 00 00  80 06 00 10 00 00 00 00  |................|
00000130  00 01 00 08 69 00 00 00  80 08 00 08 00 00 00 04  |....i...........|

As you can see I definitely set the OpenFlow version as 0x04 which means OpenFlow1.3: https://github.com/openvswitch/ovs/blob/master/include/openflow/openflow-common.h#L76

I‘m not sure if I misunderstood anything. I’m stuck here. How can I resume the pipeline in my situation?

BTW what props should be included in a NXT_RESUME? I currently just added all props that I received from packet_in2 message except NXPINT_USERDATA.

Thank you!!

blp commented 3 years ago

Thank you for the report. I see the issue. This is definitely an internal bug in Open vSwitch. Open vSwitch, like OpenFlow 1.1 and later, distinguishes between "instructions" and "actions", but OVS tends to treat the difference internally as mostly an inconvenience and tries to make them as similar as it can. In this case, there's a failure to deal with the GOTO_TABLE instruction properly. I'll see if I can fix that.

In the meantime, you can avoid the problem here by using the "resubmit" OVS extension action instead of the OF goto_table instruction. Some people might complain that they don't want to use OF extensions, but "pause/resume" are already a major extension, so I doubt it will be a problem here.

blp commented 3 years ago

I sent out a proper fix: https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/385200.html

GraysonWu commented 3 years ago

@blp Thank you so much for starting this fix so quickly. And before that, I'll see if resubmit could be a workaround in our case.