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

On 1/12/2021 3:49 PM, Richard Cochran wrote:

On Mon, Jan 11, 2021 at 08:17:48PM +0200, Eran Ben Elisha wrote:

quoted

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 <redacted>
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

quoted

+			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