From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Ricardo Leitner Subject: Re: [PATCH v3 1/2] net: create skb_gso_validate_mac_len() Date: Tue, 30 Jan 2018 06:17:02 -0200 Message-ID: <20180130081702.GJ4000@localhost.localdomain> References: <20180130011447.24916-1-dja@axtens.net> <20180130011447.24916-2-dja@axtens.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, Manish.Chopra@cavium.com, Jason Wang , Pravin Shelar To: Daniel Axtens Return-path: Received: from mail-qk0-f193.google.com ([209.85.220.193]:40286 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751521AbeA3IRW (ORCPT ); Tue, 30 Jan 2018 03:17:22 -0500 Received: by mail-qk0-f193.google.com with SMTP id d18so9038610qke.7 for ; Tue, 30 Jan 2018 00:17:22 -0800 (PST) Content-Disposition: inline In-Reply-To: <20180130011447.24916-2-dja@axtens.net> Sender: netdev-owner@vger.kernel.org List-ID: 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.