Subject: Re: [PATCH] igb: add a method to get the nic hw time stamping policy

From: Peter P Waskiewicz Jr <[email-masked]>
Date: Tue, 14 May 2013 03:51:40 -0700

On 05/14/2013 02:51 AM, Dong Zhu wrote:
> Hi,
>
>> I modified this patch and added the method to igb_get_ts_info function.
>> For 82576 nic, through this method we can easily check which type of packets
>> are time stamped now, such as (HWTSTAMP_FILTER_PTP_V1_L4_SYNC and
>> HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ), then change or disable it up to your requests.
>> I think it is convenient.The origial GET_TS_INFO method can only show the device’s
>> time stamping capabilities which nics supported.
> Through this method we can easily check which type of packets are
> timestamped currently, it is useful because that we use the hwstamp_ctl
> set time stamping policy first then we could verify what type of packets
> does the nic timestamp and then do some other testing(regression) for each
> policy.No matter PTP is running or not we can make sure whether timestamp packets
> could cause other general network problems through viewing and setting the
> timestamp policy.

I'm trying to follow this thread, and Richard's question keeps coming up 
in my head, and you've failed to answer it yet.

What are you trying to fix/solve?

It sounds like what you're trying to add can already be retrieved 
through different mechanisms that already exist.

Please explain what you're trying to accomplish that doesn't already 
exist today, and why it's important.  If the facility exists already, 
then use that.  If it's not very usable, it can (maybe) be updated given 
what's deficient.  If it doesn't exist, then explaining why it needs to 
exist will help the case for the patches.

Just providing code doesn't help answer the question of what you're 
trying to do in the first place, and why what already exists isn't good 
enough.

Regards,
-PJ