Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

max_len option for the controller action is never honored #295

Closed
antoninbas opened this issue Aug 15, 2023 · 7 comments
Closed

max_len option for the controller action is never honored #295

antoninbas opened this issue Aug 15, 2023 · 7 comments

Comments

@antoninbas
Copy link

It seems that the max_len option for the controller action is never honored, at least not with the Linux kernel datapath.

Here is my OF flow:

 cookie=0xb020000000000, duration=3663.175s, table=23, n_packets=6, n_bytes=2652, idle_age=122, hard_age=1292, priority=64991,conj_id=1 actions=controller(reason=no_match,max_len=128,id=20030,userdata=02),resubmit(,26)

Here is the datapath flow:

recirc_id(0x260),tunnel(tun_id=0x0,src=192.168.77.100,dst=192.168.77.101,flags(-df-csum+key)),in_port(1),skb_mark(0/0x80000000),ct_state(+new-est-rel-rpl-inv+trk-snat),ct_mark(0/0x7f),ct_label(0/0xffffffff00000000),eth(src=7a:af:12:8d:2a:fd,dst=aa:bb:cc:dd:ee:ff),eth_type(0x0800),ipv4(src=10.10.0.0/255.255.255.0,dst=10.10.1.11,proto=17,ttl=63,frag=no),udp(src=53), packets:0, bytes:0, used:never, actions:set(eth(src=22:46:9b:32:04:5b,dst=3e:4f:e4:c9:88:f7)),set(ipv4(ttl=62)),userspace(pid=4294967295,controller(reason=0,dont_send=0,continuation=0,recirc_id=609,rule_cookie=0xb020000000000,controller_id=20030,max_len=128)),ct(commit,zone=65520,mark=0x1/0xf),recirc(0x262)

Even though max_len is set to 128, the controller receives the full packet, which is 428B.

I looked at the code. It seems that this was meant to be implemented just before sending the packet to the controller (in connmgr), as the entire packet is always upcalled by the kernel datapath anyway.
Originally, the functionality was implemented in ofputil_encode_packet_in_private:
https://github.com/openvswitch/ovs/blob/7a0f907b2393626dac1387617355990eab69aef7/lib/ofp-util.c#L3857-L3897

The implementation was then removed in OVS v2.7 by this patch: openvswitch/ovs@c184807

And as far as I can tell, this field has been unused ever since:
https://github.com/openvswitch/ovs/blob/cf11766cbcf162399aafb84ba5634a22bccf9e8b/ofproto/connmgr.h#L49

Is this the expected behavior? Should the documentation for the controller action be updated to indicate that the max_len option may not be honored (or is never honored)?

@igsilya
Copy link
Member

igsilya commented Aug 15, 2023

Hi. It is expected behavior. According to OpenFlow spec [1]:

Switches that do not support internal buffering, are configured to not buffer
packets for the packet-in event, or have run out of internal buffering, must
send the full packet to controllers as part of the [Packet-in] event.
<...>
If the packet is buffered, the number of bytes of the original packet to include
in the packet-in can be configured. <...>

OVS reports number of supported buffers as zero in the OFPT_FEATURES_REPLY, meaning that controller cannot expect buffering. And under these conditions, the switch that doesn't support buffering (OVS) must send the whole packet in the packet-in.

The ovs-actions man page may use some update, true.

[1] Section 6.1.2 : https://opennetworking.org/wp-content/uploads/2014/10/openflow-switch-v1.5.1.pdf

@antoninbas
Copy link
Author

Thanks for the quick confirmation.

I can volunteer to update the documentation if that helps.

antoninbas added a commit to antoninbas/ofnet that referenced this issue Aug 15, 2023
Instead of 128.
65535 means that there is no buffering and that the full packet is sent
to the controller. This is actually the only value supported by OVS,
even though OVS will not reject other values. This can create confusion
as the flows will show `max_len=128`, but the controller will always
receive the full packet.
Another value for max_len can be explicitly provided using the new
`MaxLen` field in the `NXController` struct, but there should be no
reason to do so when using OVS.

See openvswitch/ovs-issues#295

Signed-off-by: Antonin Bas <[email protected]>
antoninbas added a commit to antoninbas/ofnet that referenced this issue Aug 15, 2023
Instead of 128.
65535 means that there is no buffering and that the full packet is sent
to the controller. This is actually the only value supported by OVS,
even though OVS will not reject other values. This can create confusion
as the flows will show `max_len=128`, but the controller will always
receive the full packet.
Another value for max_len can be explicitly provided using the new
`MaxLen` field in the `NXController` struct, but there should be no
reason to do so when using OVS.

See openvswitch/ovs-issues#295

Signed-off-by: Antonin Bas <[email protected]>
@igsilya
Copy link
Member

igsilya commented Aug 16, 2023

I can volunteer to update the documentation if that helps.

Thanks! Feel free to submit a patch.

antoninbas added a commit to antrea-io/ofnet that referenced this issue Aug 16, 2023
Instead of 128.
65535 means that there is no buffering and that the full packet is sent
to the controller. This is actually the only value supported by OVS,
even though OVS will not reject other values. This can create confusion
as the flows will show `max_len=128`, but the controller will always
receive the full packet.
Another value for max_len can be explicitly provided using the new
`MaxLen` field in the `NXController` struct, but there should be no
reason to do so when using OVS.

See openvswitch/ovs-issues#295

Signed-off-by: Antonin Bas <[email protected]>
@antoninbas
Copy link
Author

antoninbas commented Aug 17, 2023

ovsrobot pushed a commit to ovsrobot/ovs that referenced this issue Aug 17, 2023
Since Open vSwitch 2.7, thge max_len option has no effect, and the full
packet is always sent to controllers. This was confirmed with both the
kernel and netdev datapath.

Reported-by: Antonin Bas <[email protected]>
Reported-at: openvswitch/ovs-issues#295
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: 0-day Robot <[email protected]>
ovsrobot pushed a commit to ovsrobot/ovs that referenced this issue Aug 17, 2023
From: Antonin Bas <[email protected]>

Since Open vSwitch 2.7, the max_len option has no effect, and the full
packet is always sent to controllers. This was confirmed with both the
kernel and netdev datapaths.

Reported-by: Antonin Bas <[email protected]>
Reported-at: openvswitch/ovs-issues#295
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: 0-day Robot <[email protected]>
@antoninbas
Copy link
Author

@igsilya the patch was acked, would you mind applying it: https://mail.openvswitch.org/pipermail/ovs-dev/2023-August/407313.html ?

igsilya pushed a commit to igsilya/ovs that referenced this issue Aug 25, 2023
From: Antonin Bas <[email protected]>

Since Open vSwitch 2.7, the max_len option has no effect, and the full
packet is always sent to controllers. This was confirmed with both the
kernel and netdev datapaths.

Reported-by: Antonin Bas <[email protected]>
Reported-at: openvswitch/ovs-issues#295
Acked-by: Simon Horman <[email protected]>
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
igsilya pushed a commit to igsilya/ovs that referenced this issue Aug 25, 2023
From: Antonin Bas <[email protected]>

Since Open vSwitch 2.7, the max_len option has no effect, and the full
packet is always sent to controllers. This was confirmed with both the
kernel and netdev datapaths.

Reported-by: Antonin Bas <[email protected]>
Reported-at: openvswitch/ovs-issues#295
Acked-by: Simon Horman <[email protected]>
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
igsilya pushed a commit to igsilya/ovs that referenced this issue Aug 25, 2023
From: Antonin Bas <[email protected]>

Since Open vSwitch 2.7, the max_len option has no effect, and the full
packet is always sent to controllers. This was confirmed with both the
kernel and netdev datapaths.

Reported-by: Antonin Bas <[email protected]>
Reported-at: openvswitch/ovs-issues#295
Acked-by: Simon Horman <[email protected]>
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
igsilya pushed a commit to igsilya/ovs that referenced this issue Aug 25, 2023
From: Antonin Bas <[email protected]>

Since Open vSwitch 2.7, the max_len option has no effect, and the full
packet is always sent to controllers. This was confirmed with both the
kernel and netdev datapaths.

Reported-by: Antonin Bas <[email protected]>
Reported-at: openvswitch/ovs-issues#295
Acked-by: Simon Horman <[email protected]>
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
igsilya pushed a commit to igsilya/ovs that referenced this issue Aug 25, 2023
From: Antonin Bas <[email protected]>

Since Open vSwitch 2.7, the max_len option has no effect, and the full
packet is always sent to controllers. This was confirmed with both the
kernel and netdev datapaths.

Reported-by: Antonin Bas <[email protected]>
Reported-at: openvswitch/ovs-issues#295
Acked-by: Simon Horman <[email protected]>
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
@igsilya
Copy link
Member

igsilya commented Aug 25, 2023

Applied. Thanks!

@antoninbas
Copy link
Author

Thanks.
I confirmed that no change is required for the documentation of the output action, given that the following is not allowed:

$ ovs-ofctl mod-flows br-int 'table=0,priority=100,in_port=veth1,ip actions=output(port=controller,max_len=128)'
ovs-ofctl: output to unsupported truncate port: controller

which is already the documented behavior

roseoriorden pushed a commit to roseoriorden/ovs that referenced this issue Jul 1, 2024
From: Antonin Bas <[email protected]>

Since Open vSwitch 2.7, the max_len option has no effect, and the full
packet is always sent to controllers. This was confirmed with both the
kernel and netdev datapaths.

Reported-by: Antonin Bas <[email protected]>
Reported-at: openvswitch/ovs-issues#295
Acked-by: Simon Horman <[email protected]>
Signed-off-by: Antonin Bas <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants