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 <era...@nvidia.com>
Reviewed-by: Tariq Toukan <tar...@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