All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: Hangbin Liu <liuhangbin@gmail.com>
Cc: netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Jonathan Toppins <jtoppins@redhat.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Eric Dumazet <edumazet@google.com>,
	Miroslav Lichvar <mlichvar@redhat.com>,
	Richard Cochran <richardcochran@gmail.com>
Subject: Re: [PATCH net-next] bonding: add software timestamping support
Date: Wed, 29 Mar 2023 21:39:13 -0700	[thread overview]
Message-ID: <9644.1680151153@famine> (raw)
In-Reply-To: <ZCUJgmGacqI5Aw+L@Laptop-X1>

Hangbin Liu <liuhangbin@gmail.com> wrote:

>On Tue, Mar 28, 2023 at 08:36:58PM -0700, Jay Vosburgh wrote:
>> Hangbin Liu <liuhangbin@gmail.com> wrote:
>> 
>> >At present, bonding attempts to obtain the timestamp (ts) information of
>> >the active slave. However, this feature is only available for mode 1, 5,
>> >and 6. For other modes, bonding doesn't even provide support for software
>> >timestamping. To address this issue, let's call ethtool_op_get_ts_info
>> >when there is no primary active slave. This will enable the use of software
>> >timestamping for the bonding interface.
>> 
>> 	If I'm reading the patch below correctly, the actual functional
>> change here is to additionally set SOF_TIMESTAMPING_TX_SOFTWARE in
>> so_timestamping for the active-backup, balance-tlb and balance-alb modes
>
>No. In the description. I said for other modes, bonding doesn't even provide
>support for software timestamping. So this patch is to address this issue.
>i.e. add sw timestaming for all bonding modes.

	Ok, I think I follow now.  It is still adding only TX software
timestamping, as (from your example below) RX was already available.

	So, I do think the patch description is imprecise in saying,
"For other modes, bonding doesn't even provide support for software
timestamping" as this really refers to specifically TX timestamping.

	-J

>For mode 1,5,6. We will try find the active slave and get it's ts info
>directly. If there is no ops->get_ts_info, just use sw timestamping.
>
>For other modes, use sw timestamping directly.
>
>This is because some users want to use PTP over bond with other modes. e.g. LACP.
>They are satisfied with just sw timestamping as it's difficult to support hw
>timestamping for LACP bonding.
>
>Before this patch, bond mode with 0, 2, 3, 4 only has software-receive.
>
># ethtool -T bond0
>Time stamping parameters for bond0:
>Capabilities:
>        software-receive
>        software-system-clock
>PTP Hardware Clock: none
>Hardware Transmit Timestamp Modes: none
>Hardware Receive Filter Modes: none
>
># ptp4l -m -S -i bond0
>ptp4l[66296.154]: interface 'bond0' does not support requested timestamping mode
>failed to create a clock
>
>After this patch:
>
># ethtool -T bond0
>Time stamping parameters for bond0:
>Capabilities:
>        software-transmit
>        software-receive
>        software-system-clock
>PTP Hardware Clock: none
>Hardware Transmit Timestamp Modes: none
>Hardware Receive Filter Modes: none
>
># ptp4l -m -S -i bond0
>ptp4l[66952.474]: port 1: INITIALIZING to LISTENING on INIT_COMPLETE
>ptp4l[66952.474]: port 0: INITIALIZING to LISTENING on INIT_COMPLETE
>ptp4l[66952.474]: port 0: INITIALIZING to LISTENING on INIT_COMPLETE
>ptp4l[66981.681]: port 1: LISTENING to MASTER on ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
>ptp4l[66981.681]: selected local clock 007c50.fffe.70cdb6 as best master
>ptp4l[66981.682]: port 1: assuming the grand master role
>
>Thanks
>Hangbin

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

  reply	other threads:[~2023-03-30  4:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-29  3:13 [PATCH net-next] bonding: add software timestamping support Hangbin Liu
2023-03-29  3:36 ` Jay Vosburgh
2023-03-30  4:01   ` Hangbin Liu
2023-03-30  4:39     ` Jay Vosburgh [this message]
2023-03-29 10:27 ` Miroslav Lichvar
2023-03-30  3:33   ` Hangbin Liu
2023-03-30  4:07     ` Jakub Kicinski
2023-03-30  4:12     ` Jay Vosburgh
2023-03-31  3:32       ` Hangbin Liu
2023-04-03 10:18         ` Miroslav Lichvar
2023-04-05  9:04           ` Hangbin Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9644.1680151153@famine \
    --to=jay.vosburgh@canonical.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jtoppins@redhat.com \
    --cc=kuba@kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=mlichvar@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=richardcochran@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.