[PWCI] "[ovs-dev] netdev-native-tnl: Fix DF bit not being extracted into tunnel flags."#12
Open
[PWCI] "[ovs-dev] netdev-native-tnl: Fix DF bit not being extracted into tunnel flags."#12
Conversation
OVS adds matches on the DF/CSUM/etc bits of the tunnel info flags, but the DF bit is never actually extracted from the outer IP header during the tunnel decapsulation. This is not a huge problem, as we'll only match on what was parsed out, since matching on DF flag is not exposed through OpenFlow to the users. And since the bit is never extracted, we'll just always match on -df, which sort of "works", because the bit is never extracted and so it is never set. However, this causes misleading -df matches in the datapath flow dumps even if the packets actually have the DF bit set, which it is by default. Fix that by actually extracting the bit from the outer header while decapsulating tunneled traffic. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Signed-off-by: 0-day Robot <robot@bytheb.org>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis patch enriches tunnel metadata extraction to capture the IP "Don't Fragment" bit and fixes GRE checksum flag handling by accumulating flags rather than overwriting them, with corresponding updates to tunnel-related test fixtures. Sequence diagram for tunnel metadata extraction with DF bitsequenceDiagram
participant Packet as "dp_packet"
participant IPHeader as "ip"
participant TunnelMD as "flow_tnl"
Packet->>IPHeader: Parse IP header
IPHeader->>TunnelMD: Provide ip_tos, ip_ttl
IPHeader->>TunnelMD: Provide ip_frag_off
alt DF bit set
TunnelMD->>TunnelMD: Set FLOW_TNL_F_DONT_FRAGMENT flag
end
Sequence diagram for GRE header parsing with cumulative flagssequenceDiagram
participant Packet as "dp_packet"
participant TunnelMD as "flow_tnl"
Packet->>TunnelMD: Parse GRE header
alt GRE checksum present
TunnelMD->>TunnelMD: Add FLOW_TNL_F_CSUM to flags
end
Class diagram for updated tunnel metadata extractionclassDiagram
class flow_tnl {
+ip_tos
+ip_ttl
+flags
}
class dp_packet
class ip {
+ip_tos
+ip_ttl
+ip_frag_off
+ip_ihl_ver
}
flow_tnl <.. dp_packet : extracted from
flow_tnl <.. ip : extracted from
flow_tnl : +FLOW_TNL_F_DONT_FRAGMENT
flow_tnl : +FLOW_TNL_F_CSUM
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughUpdates tunnel metadata parsing in lib/netdev-native-tnl.c to propagate inner IPv4 DF intent and preserve existing flags when setting GRE checksum. Adjusts multiple test expectations to reflect +df-csum+key tunnel flags, revised tunnel/recirc representations, and packet_type/MPLS semantics. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed Checks (1 warning, 1 inconclusive)
✅ Passed Checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧬 Code graph analysis (1)lib/netdev-native-tnl.c (1)
🔇 Additional comments (14)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
ovsrobot
pushed a commit
that referenced
this pull request
Sep 19, 2025
When vport_add_channel() is called duplicate, the resources for previously
specified sock was not freed. This patch fixes this issue.
Reported by Address Sanitizer.
Direct leak of 60 byte(s) in 3 object(s) allocated from:
#0 0xffffb3658080 in malloc (/usr/lib64/libasan.so.6+0xa9080)
#1 0x922630 in xmalloc__ lib/util.c:141
#2 0x922718 in xmalloc lib/util.c:176
#3 0x9c67e4 in nl_sock_create lib/netlink-socket.c:147
#4 0x94cb6c in create_nl_sock lib/dpif-netlink.c:283
#5 0x950bec in dpif_netlink_port_add__ lib/dpif-netlink.c:978
#6 0x951a20 in dpif_netlink_port_add_compat lib/dpif-netlink.c:1101
#7 0x951cd0 in dpif_netlink_port_add lib/dpif-netlink.c:1147
#8 0x616354 in dpif_port_add lib/dpif.c:602
#9 0x49f424 in port_add ofproto/ofproto-dpif.c:4144
#10 0x44d51c in ofproto_port_add ofproto/ofproto.c:2204
#11 0x416914 in iface_do_create vswitchd/bridge.c:2203
#12 0x416dbc in iface_create vswitchd/bridge.c:2246
#13 0x40e1d0 in bridge_add_ports__ vswitchd/bridge.c:1225
#14 0x40e290 in bridge_add_ports vswitchd/bridge.c:1241
#15 0x40cc6c in bridge_reconfigure vswitchd/bridge.c:952
#16 0x420884 in bridge_run vswitchd/bridge.c:3440
#17 0x42f3d0 in main vswitchd/ovs-vswitchd.c:137
Fixes: 69c5158 ("dpif-netlink: don't allocate per thread netlink sockets")
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot
pushed a commit
that referenced
this pull request
Oct 9, 2025
When vport_add_channel() is called duplicate, the resources for previously
specified sock was not freed. This patch fixes this issue.
Reported by Address Sanitizer.
Direct leak of 60 byte(s) in 3 object(s) allocated from:
#0 0xffffb3658080 in malloc (/usr/lib64/libasan.so.6+0xa9080)
#1 0x922630 in xmalloc__ lib/util.c:141
#2 0x922718 in xmalloc lib/util.c:176
#3 0x9c67e4 in nl_sock_create lib/netlink-socket.c:147
#4 0x94cb6c in create_nl_sock lib/dpif-netlink.c:283
#5 0x950bec in dpif_netlink_port_add__ lib/dpif-netlink.c:978
#6 0x951a20 in dpif_netlink_port_add_compat lib/dpif-netlink.c:1101
#7 0x951cd0 in dpif_netlink_port_add lib/dpif-netlink.c:1147
#8 0x616354 in dpif_port_add lib/dpif.c:602
#9 0x49f424 in port_add ofproto/ofproto-dpif.c:4144
#10 0x44d51c in ofproto_port_add ofproto/ofproto.c:2204
#11 0x416914 in iface_do_create vswitchd/bridge.c:2203
#12 0x416dbc in iface_create vswitchd/bridge.c:2246
#13 0x40e1d0 in bridge_add_ports__ vswitchd/bridge.c:1225
#14 0x40e290 in bridge_add_ports vswitchd/bridge.c:1241
#15 0x40cc6c in bridge_reconfigure vswitchd/bridge.c:952
#16 0x420884 in bridge_run vswitchd/bridge.c:3440
#17 0x42f3d0 in main vswitchd/ovs-vswitchd.c:137
Reproduce steps:
ovs-vsctl add-br br-ovs
ovs-vsctl add-port br-ovs test -- set interface test type=internal
ip netns add ns_test
ip link set test netns ns_test
ip netns del ns_test
ifconfig br-ovs up
sleep 1
ifconfig br-ovs down
sleep 1
ovs-vsctl del-br br-ovs
Fixes: 69c5158 ("dpif-netlink: don't allocate per thread netlink sockets")
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: This is an auto submission for "[ovs-dev] netdev-native-tnl: Fix DF bit not being extracted into tunnel flags.".
See "http://patchwork.ozlabs.org/project/openvswitch/list/?series=472781" for details.
Summary by Sourcery
Improve tunnel metadata parsing by correctly handling the IPv4 DF bit and preserving existing flags when setting the GRE checksum flag, and update tests accordingly
Bug Fixes:
Tests:
Summary by CodeRabbit