| Message ID | 20230725032726.15002-1-ruc_gongyuanjun@163.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5c85f7065718a949902b238a6abd8fc907c5d3e0 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2,1/1] benet: fix return value check in be_lancer_xmit_workarounds() | expand |
On Tue, 2023-07-25 at 11:27 +0800, Yuanjun Gong wrote: > in be_lancer_xmit_workarounds(), it should go to label 'tx_drop' > if an unexpected value is returned by pskb_trim(). > > Fixes: 93040ae5cc8d ("be2net: Fix to trim skb for padded vlan packets to workaround an ASIC Bug") > Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com> > --- > drivers/net/ethernet/emulex/benet/be_main.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c > index 18c2fc880d09..0616b5fe241c 100644 > --- a/drivers/net/ethernet/emulex/benet/be_main.c > +++ b/drivers/net/ethernet/emulex/benet/be_main.c > @@ -1138,7 +1138,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter, > (lancer_chip(adapter) || BE3_chip(adapter) || > skb_vlan_tag_present(skb)) && is_ipv4_pkt(skb)) { > ip = (struct iphdr *)ip_hdr(skb); > - pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len)); > + if (unlikely(pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len)))) > + goto tx_drop; > } > > /* If vlan tag is already inlined in the packet, skip HW VLAN I'm not sure dropping the packet is the right solution here. Based on the description of the issue that this is a workaround for it might make more sense to simply put out a WARN based on the failure since it means that the tot_len field in the IP header will be modified incorrectly and a bad IPv4 checksum will be inserted.
On Tue, 2023-07-25 at 11:00 -0700, Alexander H Duyck wrote: > On Tue, 2023-07-25 at 11:27 +0800, Yuanjun Gong wrote: > > in be_lancer_xmit_workarounds(), it should go to label 'tx_drop' > > if an unexpected value is returned by pskb_trim(). > > > > Fixes: 93040ae5cc8d ("be2net: Fix to trim skb for padded vlan packets to workaround an ASIC Bug") > > Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com> > > --- > > drivers/net/ethernet/emulex/benet/be_main.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c > > index 18c2fc880d09..0616b5fe241c 100644 > > --- a/drivers/net/ethernet/emulex/benet/be_main.c > > +++ b/drivers/net/ethernet/emulex/benet/be_main.c > > @@ -1138,7 +1138,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter, > > (lancer_chip(adapter) || BE3_chip(adapter) || > > skb_vlan_tag_present(skb)) && is_ipv4_pkt(skb)) { > > ip = (struct iphdr *)ip_hdr(skb); > > - pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len)); > > + if (unlikely(pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len)))) > > + goto tx_drop; > > } > > > > /* If vlan tag is already inlined in the packet, skip HW VLAN > > I'm not sure dropping the packet is the right solution here. Based on > the description of the issue that this is a workaround for it might > make more sense to simply put out a WARN based on the failure since it > means that the tot_len field in the IP header will be modified > incorrectly and a bad IPv4 checksum will be inserted. ... which in turn means the packet will be dropped later, right? Then I guess it's better to drop it now, it requires a similar amount of code and will reduce resources usage all around. Cheers, Paolo
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Tue, 25 Jul 2023 11:27:26 +0800 you wrote: > in be_lancer_xmit_workarounds(), it should go to label 'tx_drop' > if an unexpected value is returned by pskb_trim(). > > Fixes: 93040ae5cc8d ("be2net: Fix to trim skb for padded vlan packets to workaround an ASIC Bug") > Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com> > --- > drivers/net/ethernet/emulex/benet/be_main.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Here is the summary with links: - [net,v2,1/1] benet: fix return value check in be_lancer_xmit_workarounds() https://git.kernel.org/netdev/net/c/5c85f7065718 You are awesome, thank you!
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c index 18c2fc880d09..0616b5fe241c 100644 --- a/drivers/net/ethernet/emulex/benet/be_main.c +++ b/drivers/net/ethernet/emulex/benet/be_main.c @@ -1138,7 +1138,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter, (lancer_chip(adapter) || BE3_chip(adapter) || skb_vlan_tag_present(skb)) && is_ipv4_pkt(skb)) { ip = (struct iphdr *)ip_hdr(skb); - pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len)); + if (unlikely(pskb_trim(skb, eth_hdr_len + ntohs(ip->tot_len)))) + goto tx_drop; } /* If vlan tag is already inlined in the packet, skip HW VLAN
in be_lancer_xmit_workarounds(), it should go to label 'tx_drop' if an unexpected value is returned by pskb_trim(). Fixes: 93040ae5cc8d ("be2net: Fix to trim skb for padded vlan packets to workaround an ASIC Bug") Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com> --- drivers/net/ethernet/emulex/benet/be_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)