| Message ID | ee150b21-7415-dd3f-6785-0163fd150493@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] r8169: fix accessing unset transport header | expand |
Heiner Kallweit <hkallweit1@gmail.com> : > 66e4c8d95008 ("net: warn if transport header was not set") added > a check that triggers a warning in r8169, see [0]. > > [0] https://bugzilla.kernel.org/show_bug.cgi?id=216157 > > Fixes: 8d520b4de3ed ("r8169: work around RTL8125 UDP hw bug") > Reported-by: Erhard F. <erhard_f@mailbox.org> > Tested-by: Erhard F. <erhard_f@mailbox.org> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> /me wonders... - bz216157 experiences a (2nd) warning because the rtl8169_tso_csum_v2 ARP path shares the factored read of the (unset) transport offset but said ARP path does not use the transport offset. -> ok, the warning is mostly harmless. - rtl8169_tso_csum_v2 non-ARP paths own WARN_ON_ONCE will always complain before Eric's transport specific warning triggers. -> ok, the warning is redundant. - rtl8169_features_check > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > index 3098d6672..1b7fdb4f0 100644 > --- a/drivers/net/ethernet/realtek/r8169_main.c > +++ b/drivers/net/ethernet/realtek/r8169_main.c [...] > @@ -4402,14 +4401,13 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, > struct net_device *dev, > netdev_features_t features) > { > - int transport_offset = skb_transport_offset(skb); > struct rtl8169_private *tp = netdev_priv(dev); > > if (skb_is_gso(skb)) { > if (tp->mac_version == RTL_GIGA_MAC_VER_34) > features = rtl8168evl_fix_tso(skb, features); > > - if (transport_offset > GTTCPHO_MAX && > + if (skb_transport_offset(skb) > GTTCPHO_MAX && > rtl_chip_supports_csum_v2(tp)) > features &= ~NETIF_F_ALL_TSO; > } else if (skb->ip_summed == CHECKSUM_PARTIAL) { > @@ -4420,7 +4418,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, > if (rtl_quirk_packet_padto(tp, skb)) > features &= ~NETIF_F_CSUM_MASK; > > - if (transport_offset > TCPHO_MAX && > + if (skb_transport_offset(skb) > TCPHO_MAX && > rtl_chip_supports_csum_v2(tp)) > features &= ~NETIF_F_CSUM_MASK; > } Neither skb_is_gso nor CHECKSUM_PARTIAL implies a transport header so the warning may still trigger, right ? Btw it's a bit unexpected to see a "Fixes" tag related to a RTL8125 bug as well as a "Tested-by" by the bugzilla submitter when the dmesg included in bz216157 exibits a RTL8168e/8111e.
On 04.07.2022 02:55, Francois Romieu wrote: > Heiner Kallweit <hkallweit1@gmail.com> : >> 66e4c8d95008 ("net: warn if transport header was not set") added >> a check that triggers a warning in r8169, see [0]. >> >> [0] https://bugzilla.kernel.org/show_bug.cgi?id=216157 >> >> Fixes: 8d520b4de3ed ("r8169: work around RTL8125 UDP hw bug") >> Reported-by: Erhard F. <erhard_f@mailbox.org> >> Tested-by: Erhard F. <erhard_f@mailbox.org> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > /me wonders... > > - bz216157 experiences a (2nd) warning because the rtl8169_tso_csum_v2 > ARP path shares the factored read of the (unset) transport offset > but said ARP path does not use the transport offset. > -> ok, the warning is mostly harmless. > > - rtl8169_tso_csum_v2 non-ARP paths own WARN_ON_ONCE will always > complain before Eric's transport specific warning triggers. > -> ok, the warning is redundant. > > - rtl8169_features_check > >> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c >> index 3098d6672..1b7fdb4f0 100644 >> --- a/drivers/net/ethernet/realtek/r8169_main.c >> +++ b/drivers/net/ethernet/realtek/r8169_main.c > [...] >> @@ -4402,14 +4401,13 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, >> struct net_device *dev, >> netdev_features_t features) >> { >> - int transport_offset = skb_transport_offset(skb); >> struct rtl8169_private *tp = netdev_priv(dev); >> >> if (skb_is_gso(skb)) { >> if (tp->mac_version == RTL_GIGA_MAC_VER_34) >> features = rtl8168evl_fix_tso(skb, features); >> >> - if (transport_offset > GTTCPHO_MAX && >> + if (skb_transport_offset(skb) > GTTCPHO_MAX && >> rtl_chip_supports_csum_v2(tp)) >> features &= ~NETIF_F_ALL_TSO; >> } else if (skb->ip_summed == CHECKSUM_PARTIAL) { >> @@ -4420,7 +4418,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, >> if (rtl_quirk_packet_padto(tp, skb)) >> features &= ~NETIF_F_CSUM_MASK; >> >> - if (transport_offset > TCPHO_MAX && >> + if (skb_transport_offset(skb) > TCPHO_MAX && >> rtl_chip_supports_csum_v2(tp)) >> features &= ~NETIF_F_CSUM_MASK; >> } > > Neither skb_is_gso nor CHECKSUM_PARTIAL implies a transport header so the > warning may still trigger, right ? > I'm not an expert here, and due to missing chip documentation I can't say whether the chip could handle hw csumming correctly w/o transport header. I'd see whether we get more reports of this warning. If yes, then maybe we should use skb_transport_header_was_set() explicitly and disable hw csumming if there's no transport header. > Btw it's a bit unexpected to see a "Fixes" tag related to a RTL8125 bug as > well as a "Tested-by" by the bugzilla submitter when the dmesg included in > bz216157 exibits a RTL8168e/8111e. > The Fixes tag refers to the latest change to the affected code, therefore it comes a little unexpected, right.
Heiner Kallweit <hkallweit1@gmail.com> : > On 04.07.2022 02:55, Francois Romieu wrote: > > Heiner Kallweit <hkallweit1@gmail.com> : > >> +++ b/drivers/net/ethernet/realtek/r8169_main.c > > [...] > >> @@ -4420,7 +4418,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, > >> if (rtl_quirk_packet_padto(tp, skb)) > >> features &= ~NETIF_F_CSUM_MASK; > >> > >> - if (transport_offset > TCPHO_MAX && > >> + if (skb_transport_offset(skb) > TCPHO_MAX && > >> rtl_chip_supports_csum_v2(tp)) > >> features &= ~NETIF_F_CSUM_MASK; > >> } > > > > Neither skb_is_gso nor CHECKSUM_PARTIAL implies a transport header so the > > warning may still trigger, right ? > > I'm not an expert here, and due to missing chip documentation I can't say > whether the chip could handle hw csumming correctly w/o transport header. > I'd see whether we get more reports of this warning. If yes, then maybe > we should use skb_transport_header_was_set() explicitly and disable > hw csumming if there's no transport header. (some sleep later) I had forgotten the NETIF_F_* stuff in the r8169 driver. :o/ So, yes, ignore this point. > > Btw it's a bit unexpected to see a "Fixes" tag related to a RTL8125 bug as > > well as a "Tested-by" by the bugzilla submitter when the dmesg included in > > bz216157 exibits a RTL8168e/8111e. > > > The Fixes tag refers to the latest change to the affected code, therefore > it comes a little unexpected, right. ? 8d520b4de3ed does not change the affected code. Eric's unset transport offset detection debug code would have produced the same output with the parent of the "Fixes" commit id: $ git cat-file -p 8d520b4de3ed^:drivers/net/ethernet/realtek/r8169_main.c | grep -A4 -B1 -E 'rtl8169_features_check' static netdev_features_t rtl8169_features_check(struct sk_buff *skb, struct net_device *dev, netdev_features_t features) { int transport_offset = skb_transport_offset(skb); -- .ndo_start_xmit = rtl8169_start_xmit, .ndo_features_check = rtl8169_features_check, .ndo_tx_timeout = rtl8169_tx_timeout, .ndo_validate_addr = eth_validate_addr, .ndo_change_mtu = rtl8169_change_mtu, .ndo_fix_features = rtl8169_fix_features, -> 8d520b4de3ed does not modify the first 'int transport_offset = skb_transport_offset(skb);' statement and neither does it modify the code path to rtl8169_features_check 8d520b4de3ed actually removes some logical path towards rtl8169_tso_csum_v2 but it does not change (nor does it break) the relevant code: $ git cat-file -p 8d520b4de3ed^:drivers/net/ethernet/realtek/r8169_main.c | grep -A3 -B1 -E 'bool rtl8169_tso_csum_v2' static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, struct sk_buff *skb, u32 *opts) { u32 transport_offset = (u32)skb_transport_offset(skb);
On 04.07.2022 17:40, Francois Romieu wrote: > Heiner Kallweit <hkallweit1@gmail.com> : >> On 04.07.2022 02:55, Francois Romieu wrote: >>> Heiner Kallweit <hkallweit1@gmail.com> : >>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c >>> [...] >>>> @@ -4420,7 +4418,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, >>>> if (rtl_quirk_packet_padto(tp, skb)) >>>> features &= ~NETIF_F_CSUM_MASK; >>>> >>>> - if (transport_offset > TCPHO_MAX && >>>> + if (skb_transport_offset(skb) > TCPHO_MAX && >>>> rtl_chip_supports_csum_v2(tp)) >>>> features &= ~NETIF_F_CSUM_MASK; >>>> } >>> >>> Neither skb_is_gso nor CHECKSUM_PARTIAL implies a transport header so the >>> warning may still trigger, right ? >> >> I'm not an expert here, and due to missing chip documentation I can't say >> whether the chip could handle hw csumming correctly w/o transport header. >> I'd see whether we get more reports of this warning. If yes, then maybe >> we should use skb_transport_header_was_set() explicitly and disable >> hw csumming if there's no transport header. > > (some sleep later) > > I had forgotten the NETIF_F_* stuff in the r8169 driver. :o/ > > So, yes, ignore this point. > >>> Btw it's a bit unexpected to see a "Fixes" tag related to a RTL8125 bug as >>> well as a "Tested-by" by the bugzilla submitter when the dmesg included in >>> bz216157 exibits a RTL8168e/8111e. >>> >> The Fixes tag refers to the latest change to the affected code, therefore >> it comes a little unexpected, right. > > ? > > 8d520b4de3ed does not change the affected code. > This chunk of 8d520b4de3ed @@ -4128,9 +4183,10 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, opts[1] |= transport_offset << TCPHO_SHIFT; } else { - if (unlikely(skb->len < ETH_ZLEN && rtl_test_hw_pad_bug(tp))) - /* eth_skb_pad would free the skb on error */ - return !__skb_put_padto(skb, ETH_ZLEN, false); + unsigned int padto = rtl_quirk_packet_padto(tp, skb); + + /* skb_padto would free the skb on error */ + return !__skb_put_padto(skb, padto, false); } return true; changes the context for this part of the patch. Therefore the patch wouldn't apply cleanly. @@ -4235,7 +4234,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, else WARN_ON_ONCE(1); - opts[1] |= transport_offset << TCPHO_SHIFT; + opts[1] |= skb_transport_offset(skb) << TCPHO_SHIFT; } else { unsigned int padto = rtl_quirk_packet_padto(tp, skb); > Eric's unset transport offset detection debug code would have produced the > same output with the parent of the "Fixes" commit id: > I know, but due to the fact that the warnings are harmless and the new check doesn't exist in earlier versions, I think we can omit these kernel versions. > $ git cat-file -p 8d520b4de3ed^:drivers/net/ethernet/realtek/r8169_main.c | grep -A4 -B1 -E 'rtl8169_features_check' > > static netdev_features_t rtl8169_features_check(struct sk_buff *skb, > struct net_device *dev, > netdev_features_t features) > { > int transport_offset = skb_transport_offset(skb); > -- > .ndo_start_xmit = rtl8169_start_xmit, > .ndo_features_check = rtl8169_features_check, > .ndo_tx_timeout = rtl8169_tx_timeout, > .ndo_validate_addr = eth_validate_addr, > .ndo_change_mtu = rtl8169_change_mtu, > .ndo_fix_features = rtl8169_fix_features, > > > -> 8d520b4de3ed does not modify the first > 'int transport_offset = skb_transport_offset(skb);' statement and neither > does it modify the code path to rtl8169_features_check > > 8d520b4de3ed actually removes some logical path towards rtl8169_tso_csum_v2 > but it does not change (nor does it break) the relevant code: > > $ git cat-file -p 8d520b4de3ed^:drivers/net/ethernet/realtek/r8169_main.c | grep -A3 -B1 -E 'bool rtl8169_tso_csum_v2' > > static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, > struct sk_buff *skb, u32 *opts) > { > u32 transport_offset = (u32)skb_transport_offset(skb); > >
On Mon, 2022-07-04 at 00:12 +0200, Heiner Kallweit wrote: > 66e4c8d95008 ("net: warn if transport header was not set") added > a check that triggers a warning in r8169, see [0]. > > [0] https://bugzilla.kernel.org/show_bug.cgi?id=216157 > > Fixes: 8d520b4de3ed ("r8169: work around RTL8125 UDP hw bug") > Reported-by: Erhard F. <erhard_f@mailbox.org> > Tested-by: Erhard F. <erhard_f@mailbox.org> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> The patch LGTM, but I think you could mention in the commit message that the bug was [likely] introduced with commit bdfa4ed68187 ("r8169: use Giant Send"), but this change applies only on top of the commit specified by the fixes tag - just to help stable teams. Thanks! Paolo > --- > drivers/net/ethernet/realtek/r8169_main.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > index 3098d6672..1b7fdb4f0 100644 > --- a/drivers/net/ethernet/realtek/r8169_main.c > +++ b/drivers/net/ethernet/realtek/r8169_main.c > @@ -4190,7 +4190,6 @@ static void rtl8169_tso_csum_v1(struct sk_buff *skb, u32 *opts) > static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, > struct sk_buff *skb, u32 *opts) > { > - u32 transport_offset = (u32)skb_transport_offset(skb); > struct skb_shared_info *shinfo = skb_shinfo(skb); > u32 mss = shinfo->gso_size; > > @@ -4207,7 +4206,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, > WARN_ON_ONCE(1); > } > > - opts[0] |= transport_offset << GTTCPHO_SHIFT; > + opts[0] |= skb_transport_offset(skb) << GTTCPHO_SHIFT; > opts[1] |= mss << TD1_MSS_SHIFT; > } else if (skb->ip_summed == CHECKSUM_PARTIAL) { > u8 ip_protocol; > @@ -4235,7 +4234,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, > else > WARN_ON_ONCE(1); > > - opts[1] |= transport_offset << TCPHO_SHIFT; > + opts[1] |= skb_transport_offset(skb) << TCPHO_SHIFT; > } else { > unsigned int padto = rtl_quirk_packet_padto(tp, skb); > > @@ -4402,14 +4401,13 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, > struct net_device *dev, > netdev_features_t features) > { > - int transport_offset = skb_transport_offset(skb); > struct rtl8169_private *tp = netdev_priv(dev); > > if (skb_is_gso(skb)) { > if (tp->mac_version == RTL_GIGA_MAC_VER_34) > features = rtl8168evl_fix_tso(skb, features); > > - if (transport_offset > GTTCPHO_MAX && > + if (skb_transport_offset(skb) > GTTCPHO_MAX && > rtl_chip_supports_csum_v2(tp)) > features &= ~NETIF_F_ALL_TSO; > } else if (skb->ip_summed == CHECKSUM_PARTIAL) { > @@ -4420,7 +4418,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, > if (rtl_quirk_packet_padto(tp, skb)) > features &= ~NETIF_F_CSUM_MASK; > > - if (transport_offset > TCPHO_MAX && > + if (skb_transport_offset(skb) > TCPHO_MAX && > rtl_chip_supports_csum_v2(tp)) > features &= ~NETIF_F_CSUM_MASK; > }
On 05.07.2022 14:46, Paolo Abeni wrote: > On Mon, 2022-07-04 at 00:12 +0200, Heiner Kallweit wrote: >> 66e4c8d95008 ("net: warn if transport header was not set") added >> a check that triggers a warning in r8169, see [0]. >> >> [0] https://bugzilla.kernel.org/show_bug.cgi?id=216157 >> >> Fixes: 8d520b4de3ed ("r8169: work around RTL8125 UDP hw bug") >> Reported-by: Erhard F. <erhard_f@mailbox.org> >> Tested-by: Erhard F. <erhard_f@mailbox.org> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > The patch LGTM, but I think you could mention in the commit message > that the bug was [likely] introduced with commit bdfa4ed68187 ("r8169: > use Giant Send"), but this change applies only on top of the commit > specified by the fixes tag - just to help stable teams. > Right, I'll submit a v2 with more details in the commit message. > Thanks! > > Paolo > >> --- >> drivers/net/ethernet/realtek/r8169_main.c | 10 ++++------ >> 1 file changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c >> index 3098d6672..1b7fdb4f0 100644 >> --- a/drivers/net/ethernet/realtek/r8169_main.c >> +++ b/drivers/net/ethernet/realtek/r8169_main.c >> @@ -4190,7 +4190,6 @@ static void rtl8169_tso_csum_v1(struct sk_buff *skb, u32 *opts) >> static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, >> struct sk_buff *skb, u32 *opts) >> { >> - u32 transport_offset = (u32)skb_transport_offset(skb); >> struct skb_shared_info *shinfo = skb_shinfo(skb); >> u32 mss = shinfo->gso_size; >> >> @@ -4207,7 +4206,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, >> WARN_ON_ONCE(1); >> } >> >> - opts[0] |= transport_offset << GTTCPHO_SHIFT; >> + opts[0] |= skb_transport_offset(skb) << GTTCPHO_SHIFT; >> opts[1] |= mss << TD1_MSS_SHIFT; >> } else if (skb->ip_summed == CHECKSUM_PARTIAL) { >> u8 ip_protocol; >> @@ -4235,7 +4234,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, >> else >> WARN_ON_ONCE(1); >> >> - opts[1] |= transport_offset << TCPHO_SHIFT; >> + opts[1] |= skb_transport_offset(skb) << TCPHO_SHIFT; >> } else { >> unsigned int padto = rtl_quirk_packet_padto(tp, skb); >> >> @@ -4402,14 +4401,13 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, >> struct net_device *dev, >> netdev_features_t features) >> { >> - int transport_offset = skb_transport_offset(skb); >> struct rtl8169_private *tp = netdev_priv(dev); >> >> if (skb_is_gso(skb)) { >> if (tp->mac_version == RTL_GIGA_MAC_VER_34) >> features = rtl8168evl_fix_tso(skb, features); >> >> - if (transport_offset > GTTCPHO_MAX && >> + if (skb_transport_offset(skb) > GTTCPHO_MAX && >> rtl_chip_supports_csum_v2(tp)) >> features &= ~NETIF_F_ALL_TSO; >> } else if (skb->ip_summed == CHECKSUM_PARTIAL) { >> @@ -4420,7 +4418,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, >> if (rtl_quirk_packet_padto(tp, skb)) >> features &= ~NETIF_F_CSUM_MASK; >> >> - if (transport_offset > TCPHO_MAX && >> + if (skb_transport_offset(skb) > TCPHO_MAX && >> rtl_chip_supports_csum_v2(tp)) >> features &= ~NETIF_F_CSUM_MASK; >> } >
On 05.07.2022 21:12, Jakub Kicinski wrote: > On Tue, 05 Jul 2022 14:46:14 +0200 Paolo Abeni wrote: >> On Mon, 2022-07-04 at 00:12 +0200, Heiner Kallweit wrote: >>> 66e4c8d95008 ("net: warn if transport header was not set") added >>> a check that triggers a warning in r8169, see [0]. >>> >>> [0] https://bugzilla.kernel.org/show_bug.cgi?id=216157 >>> >>> Fixes: 8d520b4de3ed ("r8169: work around RTL8125 UDP hw bug") >>> Reported-by: Erhard F. <erhard_f@mailbox.org> >>> Tested-by: Erhard F. <erhard_f@mailbox.org> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> >> The patch LGTM, but I think you could mention in the commit message >> that the bug was [likely] introduced with commit bdfa4ed68187 ("r8169: >> use Giant Send"), but this change applies only on top of the commit >> specified by the fixes tag - just to help stable teams. > > How about we put Eric's patch under Fixes? The patch is not really > needed unless the warning is there. This would also be an option. It just seemed a little illogical to me to leave the impression a new (useful) warning needs to be fixed. Just a few seconds ago I sent a v2 following Paolo's proposal.
On Tue, 5 Jul 2022 21:18:43 +0200 Heiner Kallweit wrote: > > How about we put Eric's patch under Fixes? The patch is not really > > needed unless the warning is there. > > This would also be an option. It just seemed a little illogical to me > to leave the impression a new (useful) warning needs to be fixed. > Just a few seconds ago I sent a v2 following Paolo's proposal. That's fine. Perhaps I have more utilitarian than literal view of the Fixes tag than others (how far back can the problem trigger vs blame).
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index 3098d6672..1b7fdb4f0 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -4190,7 +4190,6 @@ static void rtl8169_tso_csum_v1(struct sk_buff *skb, u32 *opts) static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, struct sk_buff *skb, u32 *opts) { - u32 transport_offset = (u32)skb_transport_offset(skb); struct skb_shared_info *shinfo = skb_shinfo(skb); u32 mss = shinfo->gso_size; @@ -4207,7 +4206,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, WARN_ON_ONCE(1); } - opts[0] |= transport_offset << GTTCPHO_SHIFT; + opts[0] |= skb_transport_offset(skb) << GTTCPHO_SHIFT; opts[1] |= mss << TD1_MSS_SHIFT; } else if (skb->ip_summed == CHECKSUM_PARTIAL) { u8 ip_protocol; @@ -4235,7 +4234,7 @@ static bool rtl8169_tso_csum_v2(struct rtl8169_private *tp, else WARN_ON_ONCE(1); - opts[1] |= transport_offset << TCPHO_SHIFT; + opts[1] |= skb_transport_offset(skb) << TCPHO_SHIFT; } else { unsigned int padto = rtl_quirk_packet_padto(tp, skb); @@ -4402,14 +4401,13 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, struct net_device *dev, netdev_features_t features) { - int transport_offset = skb_transport_offset(skb); struct rtl8169_private *tp = netdev_priv(dev); if (skb_is_gso(skb)) { if (tp->mac_version == RTL_GIGA_MAC_VER_34) features = rtl8168evl_fix_tso(skb, features); - if (transport_offset > GTTCPHO_MAX && + if (skb_transport_offset(skb) > GTTCPHO_MAX && rtl_chip_supports_csum_v2(tp)) features &= ~NETIF_F_ALL_TSO; } else if (skb->ip_summed == CHECKSUM_PARTIAL) { @@ -4420,7 +4418,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb, if (rtl_quirk_packet_padto(tp, skb)) features &= ~NETIF_F_CSUM_MASK; - if (transport_offset > TCPHO_MAX && + if (skb_transport_offset(skb) > TCPHO_MAX && rtl_chip_supports_csum_v2(tp)) features &= ~NETIF_F_CSUM_MASK; }
66e4c8d95008 ("net: warn if transport header was not set") added a check that triggers a warning in r8169, see [0]. [0] https://bugzilla.kernel.org/show_bug.cgi?id=216157 Fixes: 8d520b4de3ed ("r8169: work around RTL8125 UDP hw bug") Reported-by: Erhard F. <erhard_f@mailbox.org> Tested-by: Erhard F. <erhard_f@mailbox.org> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/ethernet/realtek/r8169_main.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)