From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Axtens Subject: Re: [PATCH v3 1/2] net: create skb_gso_validate_mac_len() Date: Wed, 31 Jan 2018 00:08:59 +1100 Message-ID: <877erzbij8.fsf@linkitivity.dja.id.au> References: <20180130011447.24916-1-dja@axtens.net> <20180130011447.24916-2-dja@axtens.net> <20180130081702.GJ4000@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain Cc: netdev@vger.kernel.org, Manish.Chopra@cavium.com, Jason Wang , Pravin Shelar To: Marcelo Ricardo Leitner Return-path: Received: from mail-pf0-f194.google.com ([209.85.192.194]:35894 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751821AbeA3NJF (ORCPT ); Tue, 30 Jan 2018 08:09:05 -0500 Received: by mail-pf0-f194.google.com with SMTP id 23so8890023pfp.3 for ; Tue, 30 Jan 2018 05:09:04 -0800 (PST) In-Reply-To: <20180130081702.GJ4000@localhost.localdomain> Sender: netdev-owner@vger.kernel.org List-ID: Marcelo Ricardo Leitner writes: > Hi, > > On Tue, Jan 30, 2018 at 12:14:46PM +1100, Daniel Axtens wrote: >> If you take a GSO skb, and split it into packets, will the MAC >> length (L2 + L3 + L4 headers + payload) of those packets be small >> enough to fit within a given length? >> >> Move skb_gso_mac_seglen() to skbuff.h with other related functions >> like skb_gso_network_seglen() so we can use it, and then create >> skb_gso_validate_mac_len to do the full calculation. >> >> Signed-off-by: Daniel Axtens >> --- >> include/linux/skbuff.h | 16 +++++++++++++ >> net/core/skbuff.c | 65 +++++++++++++++++++++++++++++++++++++++----------- >> net/sched/sch_tbf.c | 10 -------- >> 3 files changed, 67 insertions(+), 24 deletions(-) >> >> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h >> index b8e0da6c27d6..242d6773c7c2 100644 >> --- a/include/linux/skbuff.h >> +++ b/include/linux/skbuff.h >> @@ -3287,6 +3287,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen); >> void skb_scrub_packet(struct sk_buff *skb, bool xnet); >> unsigned int skb_gso_transport_seglen(const struct sk_buff *skb); >> bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu); >> +bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len); >> struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features); >> struct sk_buff *skb_vlan_untag(struct sk_buff *skb); >> int skb_ensure_writable(struct sk_buff *skb, int write_len); >> @@ -4120,6 +4121,21 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) >> return hdr_len + skb_gso_transport_seglen(skb); >> } >> >> +/** >> + * skb_gso_mac_seglen - Return length of individual segments of a gso packet >> + * >> + * @skb: GSO skb >> + * >> + * skb_gso_mac_seglen is used to determine the real size of the >> + * individual segments, including MAC/L2, Layer3 (IP, IPv6) and L4 >> + * headers (TCP/UDP). >> + */ >> +static inline unsigned int skb_gso_mac_seglen(const struct sk_buff *skb) >> +{ >> + unsigned int hdr_len = skb_transport_header(skb) - skb_mac_header(skb); >> + return hdr_len + skb_gso_transport_seglen(skb); >> +} >> + >> /* Local Checksum Offload. >> * Compute outer checksum based on the assumption that the >> * inner checksum will be offloaded later. >> diff --git a/net/core/skbuff.c b/net/core/skbuff.c >> index 01e8285aea73..55d84ab7d093 100644 >> --- a/net/core/skbuff.c >> +++ b/net/core/skbuff.c >> @@ -4914,36 +4914,73 @@ unsigned int skb_gso_transport_seglen(const struct sk_buff *skb) >> EXPORT_SYMBOL_GPL(skb_gso_transport_seglen); >> >> /** >> - * skb_gso_validate_mtu - Return in case such skb fits a given MTU >> + * skb_gso_size_check - check the skb size, considering GSO_BY_FRAGS >> * >> - * @skb: GSO skb >> - * @mtu: MTU to validate against >> + * There are a couple of instances where we have a GSO skb, and we >> + * want to determine what size it would be after it is segmented. >> * >> - * skb_gso_validate_mtu validates if a given skb will fit a wanted MTU >> - * once split. >> + * We might want to check: >> + * - L3+L4+payload size (e.g. IP forwarding) >> + * - L2+L3+L4+payload size (e.g. sanity check before passing to driver) >> + * >> + * This is a helper to do that correctly considering GSO_BY_FRAGS. >> + * >> + * @seg_len: The segmented length (from skb_gso_*_seglen). In the >> + * GSO_BY_FRAGS case this will be [header sizes + GSO_BY_FRAGS]. >> + * >> + * @max_len: The maximum permissible length. >> + * >> + * Returns true if the segmented length <= max length. >> */ >> -bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu) >> -{ >> +static inline bool skb_gso_size_check(const struct sk_buff *skb, >> + unsigned int seg_len, >> + unsigned int max_len) { >> const struct skb_shared_info *shinfo = skb_shinfo(skb); >> const struct sk_buff *iter; >> - unsigned int hlen; >> - >> - hlen = skb_gso_network_seglen(skb); >> >> if (shinfo->gso_size != GSO_BY_FRAGS) >> - return hlen <= mtu; >> + return seg_len <= max_len; >> >> /* Undo this so we can re-use header sizes */ >> - hlen -= GSO_BY_FRAGS; >> + seg_len -= GSO_BY_FRAGS; >> >> skb_walk_frags(skb, iter) { >> - if (hlen + skb_headlen(iter) > mtu) >> + if (seg_len + skb_headlen(iter) > max_len) >> return false; >> } >> >> return true; >> } >> -EXPORT_SYMBOL_GPL(skb_gso_validate_mtu); >> + >> +/** >> + * skb_gso_validate_mtu - Return in case such skb fits a given MTU >> + * >> + * @skb: GSO skb >> + * @mtu: MTU to validate against >> + * >> + * skb_gso_validate_mtu validates if a given skb will fit a wanted MTU >> + * once split. >> + */ >> +bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu) >> +{ >> + return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu); >> +} >> +EXPORT_SYMBOL_GPL(skb_gso_validate_network_len); > > This export is not matching the function name. Oh darn it! I fixed this locally and then forgot to run format-patch. Will fix it in the next spin. Thanks! Regards, Daniel