Re: [PATCH RE-SUBMIT 2/2] net/pch_gbe: supports eg20t ptp clock

On Mon, Mar 12, 2012 at 09:35:44AM +0900, Takahiro Shimizu wrote:

Hello Richard,

Thank you for the comment.

quoted

+     if ((sk_run_filter(skb, ptp_filter) != PTP_CLASS_V2_IPV4) &&

quoted

+             (sk_run_filter(skb, ptp_filter) != PTP_CLASS_V1_IPV4)) {

No, just run the filter once and test the result twice.

Also, doesn't the timestamping unit work with IPv6 and L2?  If so,
don't restrict the driver to IPv4.

According to the EG20T spec, it support IPv6 too. But I did not any test
about IPv6 now.
Do you mean I don't restrict anything here?

No, what I mean is that you only accept PTP_CLASS_V1_IPV4 or
PTP_CLASS_V2_IPV4. You should accept any packet type supported by the
hardware.

If someone tries to use your driver with PTPv2 over UDPv6, for
example, then the filter will return PTP_CLASS_V2_IPV6, and the match
will fail. It will also fail for PTPv2 L2 and PTPv2 L2/VLAN.

It does not make sense to me to limit the driver in this way.

Richard