@@ -880,7 +880,7 @@ static int fr_rx(struct sk_buff *skb)
u8 *data = skb->data;
u16 dlci;
struct pvc_device *pvc;
- struct net_device *dev = NULL;
+ struct net_device *dev;
if (skb->len <= 4 || fh->ea1 || data[2] != FR_UI)
goto rx_error;
@@ -930,13 +930,17 @@ static int fr_rx(struct sk_buff *skb)
}
if (data[3] == NLPID_IP) {
+ if (!pvc->main)
+ goto rx_drop;
skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */
- dev = pvc->main;
+ skb->dev = pvc->main;
skb->protocol = htons(ETH_P_IP);
} else if (data[3] == NLPID_IPV6) {
+ if (!pvc->main)
+ goto rx_drop;
skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */
- dev = pvc->main;
+ skb->dev = pvc->main;
skb->protocol = htons(ETH_P_IPV6);
} else if (skb->len > 10 && data[3] == FR_PAD &&
@@ -950,13 +954,16 @@ static int fr_rx(struct sk_buff *skb)
case ETH_P_IPX:
case ETH_P_IP: /* a long variant */
case ETH_P_IPV6:
- dev = pvc->main;
+ if (!pvc->main)
+ goto rx_drop;
+ skb->dev = pvc->main;
skb->protocol = htons(pid);
break;
case 0x80C20007: /* bridged Ethernet frame */
- if ((dev = pvc->ether) != NULL)
- skb->protocol = eth_type_trans(skb, dev);
+ if (!pvc->ether)
+ goto rx_drop;
+ skb->protocol = eth_type_trans(skb, pvc->ether);
break;
default:
@@ -970,17 +977,13 @@ static int fr_rx(struct sk_buff *skb)
goto rx_drop;
}
- if (dev) {
- dev->stats.rx_packets++; /* PVC traffic */
- dev->stats.rx_bytes += skb->len;
- if (pvc->state.becn)
- dev->stats.rx_compressed++;
- skb->dev = dev;
- netif_rx(skb);
- return NET_RX_SUCCESS;
- } else {
- goto rx_drop;
- }
+ dev = skb->dev;
+ dev->stats.rx_packets++; /* PVC traffic */
+ dev->stats.rx_bytes += skb->len;
+ if (pvc->state.becn)
+ dev->stats.rx_compressed++;
+ netif_rx(skb);
+ return NET_RX_SUCCESS;
rx_error:
frad->stats.rx_errors++; /* Mark error */
The eth_type_trans function is called when we receive frames carrying Ethernet frames. This function expects a non-NULL pointer as a argument, and assigns it directly to skb->dev. However, the code handling other types of frames first assigns a pointer to "dev", and then at the end checks whether the value is NULL, and if it is not NULL, assigns it to skb->dev. The two flows are different. Mixing them in this function makes the code messy. It's better that we convert the second flow to align with how eth_type_trans does things. So this patch changes the code to: first make sure the pointer is not NULL, then assign it directly to skb->dev. "dev" is no longer needed until the end where we use it to update stats. Cc: Krzysztof Halasa <khc@pm.waw.pl> Signed-off-by: Xie He <xie.he.0141@gmail.com> --- drivers/net/wan/hdlc_fr.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)