* [PATCH net-next] drivers/net/wan/hdlc_fr: Improve fr_rx and add support for any Ethertype
@ 2020-10-17 5:19 Xie He
2020-10-18 22:05 ` Jakub Kicinski
0 siblings, 1 reply; 3+ messages in thread
From: Xie He @ 2020-10-17 5:19 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, netdev, linux-kernel, Krzysztof Halasa
Cc: Xie He
1. Change the fr_rx function to make this driver support any Ethertype
when receiving. (This driver is already able to handle any Ethertype
when sending.)
Originally in the fr_rx function, the code that parses the long (10-byte)
header only recognizes a few Ethertype values and drops frames with other
Ethertype values. This patch replaces this code to make fr_rx support
any Ethertype. This patch also creates a new function fr_snap_parse as
part of the new code.
2. Change the use of the "dev" variable in fr_rx. Originally we do
"dev = something", and then at the end do "if (dev) skb->dev = dev".
Now we do "if (something) skb->dev = something", then at the end do
"dev = skb->dev".
This is to make the logic of our code consistent with eth_type_trans
(which we call). The eth_type_trans function expects a non-NULL pointer
as a parameter and assigns it directly to skb->dev.
3. Change the initial skb->len check in fr_fx from "<= 4" to "< 4".
At first we only need to ensure a 4-byte header is present. We indeed
normally need the 5th byte, too, but it'd be more logical to check its
existence when we actually need it.
Also add an fh->ea2 check to the initial checks in fr_fx. fh->ea2 == 1
means the second address byte is the final address byte. We only support
the case where the address length is 2 bytes.
4. Use "goto rx_drop" whenever we need to drop a valid frame.
Cc: Krzysztof Halasa <khc@pm.waw.pl>
Signed-off-by: Xie He <xie.he.0141@gmail.com>
---
drivers/net/wan/hdlc_fr.c | 119 +++++++++++++++++++++++---------------
1 file changed, 73 insertions(+), 46 deletions(-)
diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
index 409e5a7ad8e2..e95efc14bc97 100644
--- a/drivers/net/wan/hdlc_fr.c
+++ b/drivers/net/wan/hdlc_fr.c
@@ -871,6 +871,45 @@ static int fr_lmi_recv(struct net_device *dev, struct sk_buff *skb)
return 0;
}
+static int fr_snap_parse(struct sk_buff *skb, struct pvc_device *pvc)
+{
+ /* OUI 00-00-00 indicates an Ethertype follows */
+ if (skb->data[0] == 0x00 &&
+ skb->data[1] == 0x00 &&
+ skb->data[2] == 0x00) {
+ if (!pvc->main)
+ return -1;
+ skb->dev = pvc->main;
+ skb->protocol = *(__be16 *)(skb->data + 3); /* Ethertype */
+ skb_pull(skb, 5);
+ skb_reset_mac_header(skb);
+ return 0;
+
+ /* OUI 00-80-C2 stands for the 802.1 organization */
+ } else if (skb->data[0] == 0x00 &&
+ skb->data[1] == 0x80 &&
+ skb->data[2] == 0xC2) {
+ /* PID 00-07 stands for Ethernet frames without FCS */
+ if (skb->data[3] == 0x00 &&
+ skb->data[4] == 0x07) {
+ if (!pvc->ether)
+ return -1;
+ skb_pull(skb, 5);
+ if (skb->len < ETH_HLEN)
+ return -1;
+ skb->protocol = eth_type_trans(skb, pvc->ether);
+ return 0;
+
+ /* PID unsupported */
+ } else {
+ return -1;
+ }
+
+ /* OUI unsupported */
+ } else {
+ return -1;
+ }
+}
static int fr_rx(struct sk_buff *skb)
{
@@ -880,9 +919,9 @@ 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)
+ if (skb->len < 4 || fh->ea1 || !fh->ea2 || data[2] != FR_UI)
goto rx_error;
dlci = q922_to_dlci(skb->data);
@@ -904,8 +943,7 @@ static int fr_rx(struct sk_buff *skb)
netdev_info(frad, "No PVC for received frame's DLCI %d\n",
dlci);
#endif
- dev_kfree_skb_any(skb);
- return NET_RX_DROP;
+ goto rx_drop;
}
if (pvc->state.fecn != fh->fecn) {
@@ -931,63 +969,52 @@ 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);
+ skb_reset_mac_header(skb);
} 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);
+ skb_reset_mac_header(skb);
- } else if (skb->len > 10 && data[3] == FR_PAD &&
- data[4] == NLPID_SNAP && data[5] == FR_PAD) {
- u16 oui = ntohs(*(__be16*)(data + 6));
- u16 pid = ntohs(*(__be16*)(data + 8));
- skb_pull(skb, 10);
-
- switch ((((u32)oui) << 16) | pid) {
- case ETH_P_ARP: /* routed frame with SNAP */
- case ETH_P_IPX:
- case ETH_P_IP: /* a long variant */
- case ETH_P_IPV6:
- 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);
- break;
-
- default:
- netdev_info(frad, "Unsupported protocol, OUI=%x PID=%x\n",
- oui, pid);
- dev_kfree_skb_any(skb);
- return NET_RX_DROP;
+ } else if (data[3] == FR_PAD) {
+ if (skb->len < 5)
+ goto rx_error;
+ if (data[4] == NLPID_SNAP) { /* A SNAP header follows */
+ skb_pull(skb, 5);
+ if (skb->len < 5) /* Incomplete SNAP header */
+ goto rx_error;
+ if (fr_snap_parse(skb, pvc))
+ goto rx_drop;
+ } else {
+ goto rx_drop;
}
+
} else {
netdev_info(frad, "Unsupported protocol, NLPID=%x length=%i\n",
data[3], skb->len);
- dev_kfree_skb_any(skb);
- return NET_RX_DROP;
+ 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 {
- dev_kfree_skb_any(skb);
- return NET_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:
+rx_error:
frad->stats.rx_errors++; /* Mark error */
+rx_drop:
+ frad->stats.rx_dropped++;
dev_kfree_skb_any(skb);
return NET_RX_DROP;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net-next] drivers/net/wan/hdlc_fr: Improve fr_rx and add support for any Ethertype
2020-10-17 5:19 [PATCH net-next] drivers/net/wan/hdlc_fr: Improve fr_rx and add support for any Ethertype Xie He
@ 2020-10-18 22:05 ` Jakub Kicinski
2020-10-18 23:50 ` Xie He
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2020-10-18 22:05 UTC (permalink / raw)
To: Xie He; +Cc: David S. Miller, netdev, linux-kernel, Krzysztof Halasa
On Fri, 16 Oct 2020 22:19:51 -0700 Xie He wrote:
> 1. Change the fr_rx function to make this driver support any Ethertype
> when receiving. (This driver is already able to handle any Ethertype
> when sending.)
>
> Originally in the fr_rx function, the code that parses the long (10-byte)
> header only recognizes a few Ethertype values and drops frames with other
> Ethertype values. This patch replaces this code to make fr_rx support
> any Ethertype. This patch also creates a new function fr_snap_parse as
> part of the new code.
>
> 2. Change the use of the "dev" variable in fr_rx. Originally we do
> "dev = something", and then at the end do "if (dev) skb->dev = dev".
> Now we do "if (something) skb->dev = something", then at the end do
> "dev = skb->dev".
>
> This is to make the logic of our code consistent with eth_type_trans
> (which we call). The eth_type_trans function expects a non-NULL pointer
> as a parameter and assigns it directly to skb->dev.
>
> 3. Change the initial skb->len check in fr_fx from "<= 4" to "< 4".
> At first we only need to ensure a 4-byte header is present. We indeed
> normally need the 5th byte, too, but it'd be more logical to check its
> existence when we actually need it.
>
> Also add an fh->ea2 check to the initial checks in fr_fx. fh->ea2 == 1
> means the second address byte is the final address byte. We only support
> the case where the address length is 2 bytes.
>
> 4. Use "goto rx_drop" whenever we need to drop a valid frame.
Whenever you make a list like that it's a strong indication that
each of these should be a separate commit. That makes things easier
to review.
We have already sent a pull request for 5.10 and therefore net-next
is closed for new drivers, features, and code refactoring.
Please repost when net-next reopens after 5.10-rc1 is cut.
(http://vger.kernel.org/~davem/net-next.html will not be up to date
this time around, sorry about that).
RFC patches sent for review only are obviously welcome at any time.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] drivers/net/wan/hdlc_fr: Improve fr_rx and add support for any Ethertype
2020-10-18 22:05 ` Jakub Kicinski
@ 2020-10-18 23:50 ` Xie He
0 siblings, 0 replies; 3+ messages in thread
From: Xie He @ 2020-10-18 23:50 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Linux Kernel Network Developers, LKML, Krzysztof Halasa
On Sun, Oct 18, 2020 at 3:05 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Whenever you make a list like that it's a strong indication that
> each of these should be a separate commit. That makes things easier
> to review.
>
>
> We have already sent a pull request for 5.10 and therefore net-next
> is closed for new drivers, features, and code refactoring.
>
> Please repost when net-next reopens after 5.10-rc1 is cut.
>
> (http://vger.kernel.org/~davem/net-next.html will not be up to date
> this time around, sorry about that).
>
> RFC patches sent for review only are obviously welcome at any time.
OK. Thanks!
I'll divide this into smaller commits and repost after rc1.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-18 23:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-17 5:19 [PATCH net-next] drivers/net/wan/hdlc_fr: Improve fr_rx and add support for any Ethertype Xie He
2020-10-18 22:05 ` Jakub Kicinski
2020-10-18 23:50 ` Xie He
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).