Commit e80f40cb authored by Vladimir Oltean's avatar Vladimir Oltean Committed by David S. Miller

net: dsa: tag_8021q: replace dsa_8021q_remove_header with __skb_vlan_pop

Not only did this wheel did not need reinventing, but there is also
an issue with it: It doesn't remove the VLAN header in a way that
preserves the L2 payload checksum when that is being provided by the DSA
master hw.  It should recalculate checksum both for the push, before
removing the header, and for the pull afterwards. But the current
implementation is quite dizzying, with pulls followed immediately
afterwards by pushes, the memmove is done before the push, etc.  This
makes a DSA master with RX checksumming offload to print stack traces
with the infamous 'hw csum failure' message.

So remove the dsa_8021q_remove_header function and replace it with
something that actually works with inet checksumming.

Fixes: d4619336 ("net: dsa: tag_8021q: Create helper function for removing VLAN header")
Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 961d0e5b
...@@ -28,8 +28,6 @@ int dsa_8021q_rx_switch_id(u16 vid); ...@@ -28,8 +28,6 @@ int dsa_8021q_rx_switch_id(u16 vid);
int dsa_8021q_rx_source_port(u16 vid); int dsa_8021q_rx_source_port(u16 vid);
struct sk_buff *dsa_8021q_remove_header(struct sk_buff *skb);
#else #else
int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int index, int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int index,
...@@ -64,11 +62,6 @@ int dsa_8021q_rx_source_port(u16 vid) ...@@ -64,11 +62,6 @@ int dsa_8021q_rx_source_port(u16 vid)
return 0; return 0;
} }
struct sk_buff *dsa_8021q_remove_header(struct sk_buff *skb)
{
return NULL;
}
#endif /* IS_ENABLED(CONFIG_NET_DSA_TAG_8021Q) */ #endif /* IS_ENABLED(CONFIG_NET_DSA_TAG_8021Q) */
#endif /* _NET_DSA_8021Q_H */ #endif /* _NET_DSA_8021Q_H */
...@@ -298,47 +298,4 @@ struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev, ...@@ -298,47 +298,4 @@ struct sk_buff *dsa_8021q_xmit(struct sk_buff *skb, struct net_device *netdev,
} }
EXPORT_SYMBOL_GPL(dsa_8021q_xmit); EXPORT_SYMBOL_GPL(dsa_8021q_xmit);
/* In the DSA packet_type handler, skb->data points in the middle of the VLAN
* tag, after tpid and before tci. This is because so far, ETH_HLEN
* (DMAC, SMAC, EtherType) bytes were pulled.
* There are 2 bytes of VLAN tag left in skb->data, and upper
* layers expect the 'real' EtherType to be consumed as well.
* Coincidentally, a VLAN header is also of the same size as
* the number of bytes that need to be pulled.
*
* skb_mac_header skb->data
* | |
* v v
* | | | | | | | | | | | | | | | | | | |
* +-----------------------+-----------------------+-------+-------+-------+
* | Destination MAC | Source MAC | TPID | TCI | EType |
* +-----------------------+-----------------------+-------+-------+-------+
* ^ | |
* |<--VLAN_HLEN-->to <---VLAN_HLEN--->
* from |
* >>>>>>> v
* >>>>>>> | | | | | | | | | | | | | | |
* >>>>>>> +-----------------------+-----------------------+-------+
* >>>>>>> | Destination MAC | Source MAC | EType |
* +-----------------------+-----------------------+-------+
* ^ ^
* (now part of | |
* skb->head) skb_mac_header skb->data
*/
struct sk_buff *dsa_8021q_remove_header(struct sk_buff *skb)
{
u8 *from = skb_mac_header(skb);
u8 *dest = from + VLAN_HLEN;
memmove(dest, from, ETH_HLEN - VLAN_HLEN);
skb_pull(skb, VLAN_HLEN);
skb_push(skb, ETH_HLEN);
skb_reset_mac_header(skb);
skb_reset_mac_len(skb);
skb_pull_rcsum(skb, ETH_HLEN);
return skb;
}
EXPORT_SYMBOL_GPL(dsa_8021q_remove_header);
MODULE_LICENSE("GPL v2"); MODULE_LICENSE("GPL v2");
...@@ -250,14 +250,14 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb, ...@@ -250,14 +250,14 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
{ {
struct sja1105_meta meta = {0}; struct sja1105_meta meta = {0};
int source_port, switch_id; int source_port, switch_id;
struct vlan_ethhdr *hdr; struct ethhdr *hdr;
u16 tpid, vid, tci; u16 tpid, vid, tci;
bool is_link_local; bool is_link_local;
bool is_tagged; bool is_tagged;
bool is_meta; bool is_meta;
hdr = vlan_eth_hdr(skb); hdr = eth_hdr(skb);
tpid = ntohs(hdr->h_vlan_proto); tpid = ntohs(hdr->h_proto);
is_tagged = (tpid == ETH_P_SJA1105); is_tagged = (tpid == ETH_P_SJA1105);
is_link_local = sja1105_is_link_local(skb); is_link_local = sja1105_is_link_local(skb);
is_meta = sja1105_is_meta_frame(skb); is_meta = sja1105_is_meta_frame(skb);
...@@ -266,7 +266,12 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb, ...@@ -266,7 +266,12 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
if (is_tagged) { if (is_tagged) {
/* Normal traffic path. */ /* Normal traffic path. */
tci = ntohs(hdr->h_vlan_TCI); skb_push_rcsum(skb, ETH_HLEN);
__skb_vlan_pop(skb, &tci);
skb_pull_rcsum(skb, ETH_HLEN);
skb_reset_network_header(skb);
skb_reset_transport_header(skb);
vid = tci & VLAN_VID_MASK; vid = tci & VLAN_VID_MASK;
source_port = dsa_8021q_rx_source_port(vid); source_port = dsa_8021q_rx_source_port(vid);
switch_id = dsa_8021q_rx_switch_id(vid); switch_id = dsa_8021q_rx_switch_id(vid);
...@@ -295,12 +300,6 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb, ...@@ -295,12 +300,6 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
return NULL; return NULL;
} }
/* Delete/overwrite fake VLAN header, DSA expects to not find
* it there, see dsa_switch_rcv: skb_push(skb, ETH_HLEN).
*/
if (is_tagged)
skb = dsa_8021q_remove_header(skb);
return sja1105_rcv_meta_state_machine(skb, &meta, is_link_local, return sja1105_rcv_meta_state_machine(skb, &meta, is_link_local,
is_meta); is_meta);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment