| Message ID | 20180130011447.24916-2-dja@axtens.net |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | bnx2x: disable GSO on too-large packets | expand |
On Tue, 2018-01-30 at 12:14 +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 <dja@axtens.net> > --- > 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); > +} skb_gso_transport_seglen(skb) is quite expensive (out of line) It is unfortunate bnx2x seems to support 9600 MTU ( ETH_MAX_JUMBO_PACKET_SIZE ), because 100 bytes of headers can be too small in some cases. Presumably we could avoid calling the function for standard MTU <= 9000
On Mon, 2018-01-29 at 17:46 -0800, Eric Dumazet wrote: > On Tue, 2018-01-30 at 12:14 +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 <dja@axtens.net> > > --- > > 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); > > +} > > skb_gso_transport_seglen(skb) is quite expensive (out of line) > > It is unfortunate bnx2x seems to support 9600 MTU ( > ETH_MAX_JUMBO_PACKET_SIZE ), because 100 bytes of headers can be too > small in some cases. > > Presumably we could avoid calling the function for standard MTU <= 9000 Also are we sure about these bnx2x crashes being limited to TSO ? Maybe it will crash the same after GSO has segmented the packet and we provide a big (like 10,000 bytes) packet ? I do not believe upper stack will prevent this.
Hi Eric, >> skb_gso_transport_seglen(skb) is quite expensive (out of line) >> >> It is unfortunate bnx2x seems to support 9600 MTU ( >> ETH_MAX_JUMBO_PACKET_SIZE ), because 100 bytes of headers can be too >> small in some cases. >> >> Presumably we could avoid calling the function for standard MTU <= >> 9000 Yes, it is an expensive call. I will send another version where we only call the expensive path in the jumbo frame case. I'll wait until tomorrow in case there is any more feedback in the next 24 hours or so. > Also are we sure about these bnx2x crashes being limited to TSO ? > > Maybe it will crash the same after GSO has segmented the packet and we > provide a big (like 10,000 bytes) packet ? I do not believe upper stack > will prevent this. We did test with TSO off and that was sufficient to prevent the crash. You are right that the upper stack sends down the 10k packet, so I don't know why it prevents the crash. But we did definitely test it! I don't have access to the firmware details but I imagine it's an issue with the offloaded segmentation. Thanks for your patience with the many versions of this patch set. Regards, Daniel
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 <dja@axtens.net> > --- > 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.
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> 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 <dja@axtens.net> >> --- >> 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
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Axtens/bnx2x-disable-GSO-on-too-large-packets/20180131-000934
config: i386-randconfig-a1-201804 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
In file included from include/linux/linkage.h:7:0,
from include/linux/kernel.h:7,
from include/linux/list.h:9,
from include/linux/module.h:9,
from net/core/skbuff.c:41:
>> net/core/skbuff.c:4968:19: error: 'skb_gso_validate_network_len' undeclared here (not in a function)
EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
^
include/linux/export.h:65:16: note: in definition of macro '___EXPORT_SYMBOL'
extern typeof(sym) sym; \
^
net/core/skbuff.c:4968:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
^
vim +/skb_gso_validate_network_len +4968 net/core/skbuff.c
4954
4955 /**
4956 * skb_gso_validate_mtu - Return in case such skb fits a given MTU
4957 *
4958 * @skb: GSO skb
4959 * @mtu: MTU to validate against
4960 *
4961 * skb_gso_validate_mtu validates if a given skb will fit a wanted MTU
4962 * once split.
4963 */
4964 bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu)
4965 {
4966 return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
4967 }
> 4968 EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
4969
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Daniel,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on v4.15 next-20180126]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Axtens/bnx2x-disable-GSO-on-too-large-packets/20180131-000934
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All errors (new ones prefixed by >>):
In file included from include/linux/linkage.h:7:0,
from include/linux/kernel.h:7,
from include/linux/list.h:9,
from include/linux/module.h:9,
from net//core/skbuff.c:41:
>> net//core/skbuff.c:4968:19: error: 'skb_gso_validate_network_len' undeclared here (not in a function); did you mean 'skb_gso_validate_mac_len'?
EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
^
include/linux/export.h:65:16: note: in definition of macro '___EXPORT_SYMBOL'
extern typeof(sym) sym; \
^~~
net//core/skbuff.c:4968:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
^~~~~~~~~~~~~~~~~
vim +4968 net//core/skbuff.c
4954
4955 /**
4956 * skb_gso_validate_mtu - Return in case such skb fits a given MTU
4957 *
4958 * @skb: GSO skb
4959 * @mtu: MTU to validate against
4960 *
4961 * skb_gso_validate_mtu validates if a given skb will fit a wanted MTU
4962 * once split.
4963 */
4964 bool skb_gso_validate_mtu(const struct sk_buff *skb, unsigned int mtu)
4965 {
4966 return skb_gso_size_check(skb, skb_gso_network_seglen(skb), mtu);
4967 }
> 4968 EXPORT_SYMBOL_GPL(skb_gso_validate_network_len);
4969
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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); + +/** + * skb_gso_validate_mac_len - Will a split GSO skb fit in a given length? + * + * @skb: GSO skb + * @len: length to validate against + * + * skb_gso_validate_mac_len validates if a given skb will fit a wanted + * length once split, including L2, L3 and L4 headers and the payload. + */ +bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len) +{ + return skb_gso_size_check(skb, skb_gso_mac_seglen(skb), len); +} +EXPORT_SYMBOL_GPL(skb_gso_validate_mac_len); static struct sk_buff *skb_reorder_vlan_header(struct sk_buff *skb) { diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c index 83e76d046993..229172d509cc 100644 --- a/net/sched/sch_tbf.c +++ b/net/sched/sch_tbf.c @@ -142,16 +142,6 @@ static u64 psched_ns_t2l(const struct psched_ratecfg *r, return len; } -/* - * Return length of individual segments of a gso packet, - * including all headers (MAC, IP, TCP/UDP) - */ -static 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); -} - /* GSO packet is too big, segment it so that tbf can transmit * each segment in time */
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 <dja@axtens.net> --- include/linux/skbuff.h | 16 +++++++++++++ net/core/skbuff.c | 65 +++++++++++++++++++++++++++++++++++++++----------- net/sched/sch_tbf.c | 10 -------- 3 files changed, 67 insertions(+), 24 deletions(-)