Skip to content
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.

Commit

Permalink
Merge branch 'vlan_get_protocol'
Browse files Browse the repository at this point in the history
Toshiaki Makita says:

====================
Fix checksum error when using stacked vlan

When I was testing 802.1ad, I found several drivers don't take into
account 802.1ad or multiple vlans when retrieving L3 (IP/IPv6) or
L4 (TCP/UDP) protocol for checksum offload.

It is mainly due to vlan_get_protocol(), which extracts ether type only
when it is tagged with single 802.1Q. When 802.1ad is used or there are
multiple vlans, it extracts vlan protocol and drivers cannot determine
which L3/L4 protocol is used.

Those drivers, most of which have IP_CSUM/IPV6_CSUM features, get L3/L4
header-offset by software, so it seems that their checksum offload works
with multiple vlans if we can parse protocols correctly.
(They know mac header length, and probably don't care about what is in it.)

And another thing, some of Intel's drivers seem to use skb->protocol where
vlan_get_protocol() is more suitable.

I tested that at least igb/igbvf on I350 works with this patch set.

Note:
We can hand a double tagged packet with CHECKSUM_PARTIAL to a HW driver
by creating a vlan device on a bridge device and enabling vlan_filtering
of the bridge with 802.1ad protocol.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
davem330 committed Jan 31, 2015
2 parents cfbf654 + 10e4fb3 commit 08178e5
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 55 deletions.
19 changes: 11 additions & 8 deletions drivers/net/ethernet/intel/igbvf/netdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -1907,7 +1907,8 @@ static void igbvf_watchdog_task(struct work_struct *work)

static int igbvf_tso(struct igbvf_adapter *adapter,
struct igbvf_ring *tx_ring,
struct sk_buff *skb, u32 tx_flags, u8 *hdr_len)
struct sk_buff *skb, u32 tx_flags, u8 *hdr_len,
__be16 protocol)
{
struct e1000_adv_tx_context_desc *context_desc;
struct igbvf_buffer *buffer_info;
Expand All @@ -1927,7 +1928,7 @@ static int igbvf_tso(struct igbvf_adapter *adapter,
l4len = tcp_hdrlen(skb);
*hdr_len += l4len;

if (skb->protocol == htons(ETH_P_IP)) {
if (protocol == htons(ETH_P_IP)) {
struct iphdr *iph = ip_hdr(skb);
iph->tot_len = 0;
iph->check = 0;
Expand Down Expand Up @@ -1958,7 +1959,7 @@ static int igbvf_tso(struct igbvf_adapter *adapter,
/* ADV DTYP TUCMD MKRLOC/ISCSIHEDLEN */
tu_cmd |= (E1000_TXD_CMD_DEXT | E1000_ADVTXD_DTYP_CTXT);

if (skb->protocol == htons(ETH_P_IP))
if (protocol == htons(ETH_P_IP))
tu_cmd |= E1000_ADVTXD_TUCMD_IPV4;
tu_cmd |= E1000_ADVTXD_TUCMD_L4T_TCP;

Expand All @@ -1984,7 +1985,8 @@ static int igbvf_tso(struct igbvf_adapter *adapter,

static inline bool igbvf_tx_csum(struct igbvf_adapter *adapter,
struct igbvf_ring *tx_ring,
struct sk_buff *skb, u32 tx_flags)
struct sk_buff *skb, u32 tx_flags,
__be16 protocol)
{
struct e1000_adv_tx_context_desc *context_desc;
unsigned int i;
Expand All @@ -2011,7 +2013,7 @@ static inline bool igbvf_tx_csum(struct igbvf_adapter *adapter,
tu_cmd |= (E1000_TXD_CMD_DEXT | E1000_ADVTXD_DTYP_CTXT);

if (skb->ip_summed == CHECKSUM_PARTIAL) {
switch (skb->protocol) {
switch (protocol) {
case htons(ETH_P_IP):
tu_cmd |= E1000_ADVTXD_TUCMD_IPV4;
if (ip_hdr(skb)->protocol == IPPROTO_TCP)
Expand Down Expand Up @@ -2211,6 +2213,7 @@ static netdev_tx_t igbvf_xmit_frame_ring_adv(struct sk_buff *skb,
u8 hdr_len = 0;
int count = 0;
int tso = 0;
__be16 protocol = vlan_get_protocol(skb);

if (test_bit(__IGBVF_DOWN, &adapter->state)) {
dev_kfree_skb_any(skb);
Expand Down Expand Up @@ -2239,21 +2242,21 @@ static netdev_tx_t igbvf_xmit_frame_ring_adv(struct sk_buff *skb,
tx_flags |= (vlan_tx_tag_get(skb) << IGBVF_TX_FLAGS_VLAN_SHIFT);
}

if (skb->protocol == htons(ETH_P_IP))
if (protocol == htons(ETH_P_IP))
tx_flags |= IGBVF_TX_FLAGS_IPV4;

first = tx_ring->next_to_use;

tso = skb_is_gso(skb) ?
igbvf_tso(adapter, tx_ring, skb, tx_flags, &hdr_len) : 0;
igbvf_tso(adapter, tx_ring, skb, tx_flags, &hdr_len, protocol) : 0;
if (unlikely(tso < 0)) {
dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}

if (tso)
tx_flags |= IGBVF_TX_FLAGS_TSO;
else if (igbvf_tx_csum(adapter, tx_ring, skb, tx_flags) &&
else if (igbvf_tx_csum(adapter, tx_ring, skb, tx_flags, protocol) &&
(skb->ip_summed == CHECKSUM_PARTIAL))
tx_flags |= IGBVF_TX_FLAGS_CSUM;

Expand Down
2 changes: 1 addition & 1 deletion drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -7227,11 +7227,11 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
if (!vhdr)
goto out_drop;

protocol = vhdr->h_vlan_encapsulated_proto;
tx_flags |= ntohs(vhdr->h_vlan_TCI) <<
IXGBE_TX_FLAGS_VLAN_SHIFT;
tx_flags |= IXGBE_TX_FLAGS_SW_VLAN;
}
protocol = vlan_get_protocol(skb);

if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
adapter->ptp_clock &&
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -3099,7 +3099,7 @@ static int ixgbevf_tso(struct ixgbevf_ring *tx_ring,
/* ADV DTYP TUCMD MKRLOC/ISCSIHEDLEN */
type_tucmd = IXGBE_ADVTXD_TUCMD_L4T_TCP;

if (skb->protocol == htons(ETH_P_IP)) {
if (first->protocol == htons(ETH_P_IP)) {
struct iphdr *iph = ip_hdr(skb);
iph->tot_len = 0;
iph->check = 0;
Expand Down Expand Up @@ -3156,7 +3156,7 @@ static void ixgbevf_tx_csum(struct ixgbevf_ring *tx_ring,

if (skb->ip_summed == CHECKSUM_PARTIAL) {
u8 l4_hdr = 0;
switch (skb->protocol) {
switch (first->protocol) {
case htons(ETH_P_IP):
vlan_macip_lens |= skb_network_header_len(skb);
type_tucmd |= IXGBE_ADVTXD_TUCMD_IPV4;
Expand Down
60 changes: 46 additions & 14 deletions include/linux/if_vlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -472,27 +472,59 @@ static inline int vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci)
/**
* vlan_get_protocol - get protocol EtherType.
* @skb: skbuff to query
* @type: first vlan protocol
* @depth: buffer to store length of eth and vlan tags in bytes
*
* Returns the EtherType of the packet, regardless of whether it is
* vlan encapsulated (normal or hardware accelerated) or not.
*/
static inline __be16 vlan_get_protocol(const struct sk_buff *skb)
static inline __be16 __vlan_get_protocol(struct sk_buff *skb, __be16 type,
int *depth)
{
__be16 protocol = 0;

if (vlan_tx_tag_present(skb) ||
skb->protocol != cpu_to_be16(ETH_P_8021Q))
protocol = skb->protocol;
else {
__be16 proto, *protop;
protop = skb_header_pointer(skb, offsetof(struct vlan_ethhdr,
h_vlan_encapsulated_proto),
sizeof(proto), &proto);
if (likely(protop))
protocol = *protop;
unsigned int vlan_depth = skb->mac_len;

/* if type is 802.1Q/AD then the header should already be
* present at mac_len - VLAN_HLEN (if mac_len > 0), or at
* ETH_HLEN otherwise
*/
if (type == htons(ETH_P_8021Q) || type == htons(ETH_P_8021AD)) {
if (vlan_depth) {
if (WARN_ON(vlan_depth < VLAN_HLEN))
return 0;
vlan_depth -= VLAN_HLEN;
} else {
vlan_depth = ETH_HLEN;
}
do {
struct vlan_hdr *vh;

if (unlikely(!pskb_may_pull(skb,
vlan_depth + VLAN_HLEN)))
return 0;

vh = (struct vlan_hdr *)(skb->data + vlan_depth);
type = vh->h_vlan_encapsulated_proto;
vlan_depth += VLAN_HLEN;
} while (type == htons(ETH_P_8021Q) ||
type == htons(ETH_P_8021AD));
}

return protocol;
if (depth)
*depth = vlan_depth;

return type;
}

/**
* vlan_get_protocol - get protocol EtherType.
* @skb: skbuff to query
*
* Returns the EtherType of the packet, regardless of whether it is
* vlan encapsulated (normal or hardware accelerated) or not.
*/
static inline __be16 vlan_get_protocol(struct sk_buff *skb)
{
return __vlan_get_protocol(skb, skb->protocol, NULL);
}

static inline void vlan_set_encap_proto(struct sk_buff *skb,
Expand Down
31 changes: 1 addition & 30 deletions net/core/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -2352,7 +2352,6 @@ EXPORT_SYMBOL(skb_checksum_help);

__be16 skb_network_protocol(struct sk_buff *skb, int *depth)
{
unsigned int vlan_depth = skb->mac_len;
__be16 type = skb->protocol;

/* Tunnel gso handlers can set protocol to ethernet. */
Expand All @@ -2366,35 +2365,7 @@ __be16 skb_network_protocol(struct sk_buff *skb, int *depth)
type = eth->h_proto;
}

/* if skb->protocol is 802.1Q/AD then the header should already be
* present at mac_len - VLAN_HLEN (if mac_len > 0), or at
* ETH_HLEN otherwise
*/
if (type == htons(ETH_P_8021Q) || type == htons(ETH_P_8021AD)) {
if (vlan_depth) {
if (WARN_ON(vlan_depth < VLAN_HLEN))
return 0;
vlan_depth -= VLAN_HLEN;
} else {
vlan_depth = ETH_HLEN;
}
do {
struct vlan_hdr *vh;

if (unlikely(!pskb_may_pull(skb,
vlan_depth + VLAN_HLEN)))
return 0;

vh = (struct vlan_hdr *)(skb->data + vlan_depth);
type = vh->h_vlan_encapsulated_proto;
vlan_depth += VLAN_HLEN;
} while (type == htons(ETH_P_8021Q) ||
type == htons(ETH_P_8021AD));
}

*depth = vlan_depth;

return type;
return __vlan_get_protocol(skb, type, depth);
}

/**
Expand Down

0 comments on commit 08178e5

Please sign in to comment.