diff mbox series

[bpf-next,1/2] bpf: Update h_proto of ethhdr when the outer protocol changed

Message ID 70fc4e7bf2c760b045898b3d004a0838902f7e08.1691639830.git.william.xuanziyang@huawei.com (mailing list archive)
State Rejected
Delegated to: BPF
Headers show
Series bpf: Update h_proto of ethhdr when the outer protocol changed | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain_full }}
bpf/vmtest-bpf-next-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 fail Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 fail Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-5 fail Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 fail Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-8 success Logs for veristat
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1354 this patch: 1354
netdev/cc_maintainers warning 5 maintainers not CCed: kuba@kernel.org netdev@vger.kernel.org davem@davemloft.net pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 1351 this patch: 1351
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1377 this patch: 1377
netdev/checkpatch warning WARNING: line length of 86 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline fail Was 0 now: 1

Commit Message

Ziyang Xuan (William) Aug. 10, 2023, 6:25 a.m. UTC 
When use bpf_skb_adjust_room() to encapsulate or decapsulate packet,
and outer protocol changed, we can update h_proto of ethhdr directly.

Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 net/core/filter.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

Comments

Martin KaFai Lau Aug. 10, 2023, 6:27 p.m. UTC | #1 
On 8/9/23 11:25 PM, Ziyang Xuan wrote:
> When use bpf_skb_adjust_room() to encapsulate or decapsulate packet,
> and outer protocol changed, we can update h_proto of ethhdr directly.

This could break some existing bpf programs. e.g what if the existing prog is 
testing the h_proto after bpf_skb_adjust_room() and expect it hasn't changed yet?
Ziyang Xuan (William) Aug. 11, 2023, 10:22 a.m. UTC | #2 
> On 8/9/23 11:25 PM, Ziyang Xuan wrote:
>> When use bpf_skb_adjust_room() to encapsulate or decapsulate packet,
>> and outer protocol changed, we can update h_proto of ethhdr directly.
> 
> This could break some existing bpf programs. e.g what if the existing prog is testing the h_proto after bpf_skb_adjust_room() and expect it hasn't changed yet?
> 
I think some new modifications break some existing things are not unacceptable.
Maybe my modification is inappropriate because its benefits are small and
some kind of principle is broken, such as Yonghong Song pointed:
"bpf_skb_adjust_room() only changes skb meta data and tries not to modify the packet."
If it is, the modification should be rejected.

Thank you!
William Xuan
> 
> .
diff mbox series

Patch

diff --git a/net/core/filter.c b/net/core/filter.c
index a094694899c9..0c156550c757 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3409,6 +3409,14 @@  static u32 bpf_skb_net_base_len(const struct sk_buff *skb)
 					  BPF_ADJ_ROOM_ENCAP_L2_MASK) | \
 					 BPF_F_ADJ_ROOM_DECAP_L3_MASK)
 
+static inline void skb_update_protocol(struct sk_buff *skb, __be16 protocol)
+{
+	struct ethhdr *hdr = eth_hdr(skb);
+
+	hdr->h_proto = protocol;
+	skb->protocol = protocol;
+}
+
 static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
 			    u64 flags)
 {
@@ -3491,13 +3499,13 @@  static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
 			skb_set_transport_header(skb, mac_len + nh_len);
 		}
 
-		/* Match skb->protocol to new outer l3 protocol */
+		/* Match skb->protocol and ethhdr->h_proto to new outer l3 protocol */
 		if (skb->protocol == htons(ETH_P_IP) &&
 		    flags & BPF_F_ADJ_ROOM_ENCAP_L3_IPV6)
-			skb->protocol = htons(ETH_P_IPV6);
+			skb_update_protocol(skb, htons(ETH_P_IPV6));
 		else if (skb->protocol == htons(ETH_P_IPV6) &&
 			 flags & BPF_F_ADJ_ROOM_ENCAP_L3_IPV4)
-			skb->protocol = htons(ETH_P_IP);
+			skb_update_protocol(skb, htons(ETH_P_IP));
 	}
 
 	if (skb_is_gso(skb)) {
@@ -3540,13 +3548,13 @@  static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
 	if (unlikely(ret < 0))
 		return ret;
 
-	/* Match skb->protocol to new outer l3 protocol */
+	/* Match skb->protocol and ethhdr->h_proto to new outer l3 protocol */
 	if (skb->protocol == htons(ETH_P_IP) &&
 	    flags & BPF_F_ADJ_ROOM_DECAP_L3_IPV6)
-		skb->protocol = htons(ETH_P_IPV6);
+		skb_update_protocol(skb, htons(ETH_P_IPV6));
 	else if (skb->protocol == htons(ETH_P_IPV6) &&
 		 flags & BPF_F_ADJ_ROOM_DECAP_L3_IPV4)
-		skb->protocol = htons(ETH_P_IP);
+		skb_update_protocol(skb, htons(ETH_P_IP));
 
 	if (skb_is_gso(skb)) {
 		struct skb_shared_info *shinfo = skb_shinfo(skb);