[PATCH net-next v3 0/2] Dissect PTP L2 packet header

[PATCH net-next v3 0/2] Dissect PTP L2 packet header

[Eran Ben Elisha]

Hi Jakub, Dave,

This series adds support for dissecting PTP L2 packet
header (EtherType 0x88F7).

For packet header dissecting, skb->protocol is needed. Add protocol
parsing operation to vlan ops, to guarantee skb->protocol is set,
as EtherType 0x88F7 occasionally follows a vlan header.

Changelog:
v2, v3:
- Add more people to the CC list.

Eran Ben Elisha (2):
  net: vlan: Add parse protocol header ops
  net: flow_dissector: Parse PTP L2 packet header

 net/8021q/vlan_dev.c      |  9 +++++++++
 net/core/flow_dissector.c | 16 ++++++++++++++++
 2 files changed, 25 insertions(+)

-- 
2.17.1

[PATCH net-next v3 1/2] net: vlan: Add parse protocol header ops

[Eran Ben Elisha]

Add parse protocol header ops for vlan device. Before this patch, vlan
tagged packet transmitted by af_packet had skb->protocol unset. Some
kernel methods (like __skb_flow_dissect()) rely on this missing information
for its packet processing.

Signed-off-by: Eran Ben Elisha <eranbe@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
---
 net/8021q/vlan_dev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index ec8408d1638f..dc1a197792e6 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -510,9 +510,17 @@ static void vlan_dev_set_lockdep_class(struct net_device *dev)
 	netdev_for_each_tx_queue(dev, vlan_dev_set_lockdep_one, NULL);
 }
 
+static __be16 vlan_parse_protocol(const struct sk_buff *skb)
+{
+	struct vlan_ethhdr *veth = (struct vlan_ethhdr *)(skb->data);
+
+	return __vlan_get_protocol(skb, veth->h_vlan_proto, NULL);
+}
+
 static const struct header_ops vlan_header_ops = {
 	.create	 = vlan_dev_hard_header,
 	.parse	 = eth_header_parse,
+	.parse_protocol = vlan_parse_protocol,
 };
 
 static int vlan_passthru_hard_header(struct sk_buff *skb, struct net_device *dev,
@@ -532,6 +540,7 @@ static int vlan_passthru_hard_header(struct sk_buff *skb, struct net_device *dev
 static const struct header_ops vlan_passthru_header_ops = {
 	.create	 = vlan_passthru_hard_header,
 	.parse	 = eth_header_parse,
+	.parse_protocol = vlan_parse_protocol,
 };
 
 static struct device_type vlan_type = {
-- 
2.17.1

[PATCH net-next v3 2/2] net: flow_dissector: Parse PTP L2 packet header

[Eran Ben Elisha]

Add support for parsing PTP L2 packet header. Such packet consists
of an L2 header (with ethertype of ETH_P_1588), PTP header, body
and an optional suffix.

Signed-off-by: Eran Ben Elisha <eranbe@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
---
 net/core/flow_dissector.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 6f1adba6695f..fcaa223c7cdc 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -23,6 +23,7 @@
 #include <linux/if_ether.h>
 #include <linux/mpls.h>
 #include <linux/tcp.h>
+#include <linux/ptp_classify.h>
 #include <net/flow_dissector.h>
 #include <scsi/fc/fc_fcoe.h>
 #include <uapi/linux/batadv_packet.h>
@@ -1251,6 +1252,21 @@ bool __skb_flow_dissect(const struct net *net,
 						  &proto, &nhoff, hlen, flags);
 		break;
 
+	case htons(ETH_P_1588): {
+		struct ptp_header *hdr, _hdr;
+
+		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
+					   hlen, &_hdr);
+		if (!hdr || (hlen - nhoff) < sizeof(_hdr)) {
+			fdret = FLOW_DISSECT_RET_OUT_BAD;
+			break;
+		}
+
+		nhoff += ntohs(hdr->message_length);
+		fdret = FLOW_DISSECT_RET_OUT_GOOD;
+		break;
+	}
+
 	default:
 		fdret = FLOW_DISSECT_RET_OUT_BAD;
 		break;
-- 
2.17.1

Re: [PATCH net-next v3 2/2] net: flow_dissector: Parse PTP L2 packet header

[Richard Cochran]

On Mon, Jan 11, 2021 at 08:17:48PM +0200, Eran Ben Elisha wrote:
> Add support for parsing PTP L2 packet header. Such packet consists
> of an L2 header (with ethertype of ETH_P_1588), PTP header, body
> and an optional suffix.
> 
> Signed-off-by: Eran Ben Elisha <eranbe@nvidia.com>
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> ---
>  net/core/flow_dissector.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 6f1adba6695f..fcaa223c7cdc 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -23,6 +23,7 @@
>  #include <linux/if_ether.h>
>  #include <linux/mpls.h>
>  #include <linux/tcp.h>
> +#include <linux/ptp_classify.h>
>  #include <net/flow_dissector.h>
>  #include <scsi/fc/fc_fcoe.h>
>  #include <uapi/linux/batadv_packet.h>
> @@ -1251,6 +1252,21 @@ bool __skb_flow_dissect(const struct net *net,
>  						  &proto, &nhoff, hlen, flags);
>  		break;
>  
> +	case htons(ETH_P_1588): {
> +		struct ptp_header *hdr, _hdr;
> +
> +		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
> +					   hlen, &_hdr);
> +		if (!hdr || (hlen - nhoff) < sizeof(_hdr)) {

I'm not really familiar with the flow dissector, but why check (hlen - nhoff) here?
None of the other cases do that.  Doesn't skb_copy_bits() in
__skb_header_pointer() already handle that?

Thanks,
Richard


> +			fdret = FLOW_DISSECT_RET_OUT_BAD;
> +			break;
> +		}
> +
> +		nhoff += ntohs(hdr->message_length);
> +		fdret = FLOW_DISSECT_RET_OUT_GOOD;
> +		break;
> +	}
> +
>  	default:
>  		fdret = FLOW_DISSECT_RET_OUT_BAD;
>  		break;
> -- 
> 2.17.1
> 

Re: [PATCH net-next v3 2/2] net: flow_dissector: Parse PTP L2 packet header

[Eran Ben Elisha]

On 1/12/2021 3:49 PM, Richard Cochran wrote:
> On Mon, Jan 11, 2021 at 08:17:48PM +0200, Eran Ben Elisha wrote:
>> Add support for parsing PTP L2 packet header. Such packet consists
>> of an L2 header (with ethertype of ETH_P_1588), PTP header, body
>> and an optional suffix.
>>
>> Signed-off-by: Eran Ben Elisha <eranbe@nvidia.com>
>> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
>> ---
>>   net/core/flow_dissector.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index 6f1adba6695f..fcaa223c7cdc 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> @@ -23,6 +23,7 @@
>>   #include <linux/if_ether.h>
>>   #include <linux/mpls.h>
>>   #include <linux/tcp.h>
>> +#include <linux/ptp_classify.h>
>>   #include <net/flow_dissector.h>
>>   #include <scsi/fc/fc_fcoe.h>
>>   #include <uapi/linux/batadv_packet.h>
>> @@ -1251,6 +1252,21 @@ bool __skb_flow_dissect(const struct net *net,
>>   						  &proto, &nhoff, hlen, flags);
>>   		break;
>>   
>> +	case htons(ETH_P_1588): {
>> +		struct ptp_header *hdr, _hdr;
>> +
>> +		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
>> +					   hlen, &_hdr);
>> +		if (!hdr || (hlen - nhoff) < sizeof(_hdr)) {
> 
> I'm not really familiar with the flow dissector, but why check (hlen - nhoff) here?
> None of the other cases do that.  Doesn't skb_copy_bits() in
> __skb_header_pointer() already handle that?

You are right. I saw similar len validation at ETH_P_FCOE case. But now 
I see it does not go through __skb_header_pointer() flow.

Thanks.

> 
> Thanks,
> Richard
> 
> 
>> +			fdret = FLOW_DISSECT_RET_OUT_BAD;
>> +			break;
>> +		}
>> +
>> +		nhoff += ntohs(hdr->message_length);
>> +		fdret = FLOW_DISSECT_RET_OUT_GOOD;
>> +		break;
>> +	}
>> +
>>   	default:
>>   		fdret = FLOW_DISSECT_RET_OUT_BAD;
>>   		break;
>> -- 
>> 2.17.1
>>