| Message ID | 20241105174403.850330-2-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1e4033b53db48cc77c436feac9c6d7d63db87b87 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: add debug checks to skb_reset_xxx_header() | expand |
On Tue, Nov 05, 2024 at 05:43:57PM +0000, Eric Dumazet wrote: > Recent discussions show that skb_reset_mac_len() should be more careful. > > We expect the MAC header being set. > > If not, clear skb->mac_len and fire a warning for CONFIG_DEBUG_NET=y builds. > > If after investigations we find that not having a MAC header was okay, > we can remove the warning. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Link: https://lore.kernel.org/netdev/CANn89iJZGH+yEfJxfPWa3Hm7jxb-aeY2Up4HufmLMnVuQXt38A@mail.gmail.com/T/ > Cc: En-Wei Wu <en-wei.wu@canonical.com> > --- > include/linux/skbuff.h | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) Reviewed-by: Joe Damato <jdamato@fastly.com>
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 48f1e0fa2a13619e41dfba40f2593dd61f9b9a06..5d8fefa71cac78d83b9565d9038c319112da1e2d 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -2909,9 +2909,19 @@ static inline void skb_reset_inner_headers(struct sk_buff *skb) skb->inner_transport_header = skb->transport_header; } +static inline int skb_mac_header_was_set(const struct sk_buff *skb) +{ + return skb->mac_header != (typeof(skb->mac_header))~0U; +} + static inline void skb_reset_mac_len(struct sk_buff *skb) { - skb->mac_len = skb->network_header - skb->mac_header; + if (!skb_mac_header_was_set(skb)) { + DEBUG_NET_WARN_ON_ONCE(1); + skb->mac_len = 0; + } else { + skb->mac_len = skb->network_header - skb->mac_header; + } } static inline unsigned char *skb_inner_transport_header(const struct sk_buff @@ -3014,11 +3024,6 @@ static inline void skb_set_network_header(struct sk_buff *skb, const int offset) skb->network_header += offset; } -static inline int skb_mac_header_was_set(const struct sk_buff *skb) -{ - return skb->mac_header != (typeof(skb->mac_header))~0U; -} - static inline unsigned char *skb_mac_header(const struct sk_buff *skb) { DEBUG_NET_WARN_ON_ONCE(!skb_mac_header_was_set(skb));
Recent discussions show that skb_reset_mac_len() should be more careful. We expect the MAC header being set. If not, clear skb->mac_len and fire a warning for CONFIG_DEBUG_NET=y builds. If after investigations we find that not having a MAC header was okay, we can remove the warning. Signed-off-by: Eric Dumazet <edumazet@google.com> Link: https://lore.kernel.org/netdev/CANn89iJZGH+yEfJxfPWa3Hm7jxb-aeY2Up4HufmLMnVuQXt38A@mail.gmail.com/T/ Cc: En-Wei Wu <en-wei.wu@canonical.com> --- include/linux/skbuff.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)