| Message ID | 20220310134830.130818-1-sunmingbao@tom.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tcp: export symbol tcp_set_congestion_control | expand |
Please submit this together with the actual user(s) in a series. Patches to just export random symbols are a no-go.
On Thu, 10 Mar 2022 15:11:35 +0100 Christoph Hellwig <hch@lst.de> wrote: > Please submit this together with the actual user(s) in a series. > Patches to just export random symbols are a no-go. Got it. many thanks for informing. BTW: could you give me the answer to the following questions? 1. Against which repo should I prepare and test this series of patches? netdev or nvme? 2. what's the recipients for this series of patches? netdev or nvme or both?
On Thu, Mar 10, 2022 at 11:03:00PM +0800, Mingbao Sun wrote: > On Thu, 10 Mar 2022 15:11:35 +0100 > Christoph Hellwig <hch@lst.de> wrote: > > > Please submit this together with the actual user(s) in a series. > > Patches to just export random symbols are a no-go. > > Got it. > many thanks for informing. > > BTW: > could you give me the answer to the following questions? > 1. Against which repo should I prepare and test this series of patches? > netdev or nvme? I think for the initial RFC either is fine, just clearly state which. > 2. what's the recipients for this series of patches? > netdev or nvme or both? both.
On Thu, 10 Mar 2022 21:48:30 +0800 Mingbao Sun wrote: > Since the kernel API 'kernel_setsockopt' was removed, and since the > function ‘tcp_set_congestion_control’ is just the real underlying guy > handling this job, so it makes sense to get it exported. Do you happen to have a reference to the commit which removed kernel_setsockopt and the justification? My knee jerk reaction would the that's a better path than allowing in-kernel socket users to poke at internal functions even if that works as of today.
On Thu, 10 Mar 2022 12:48:25 -0800 Jakub Kicinski <kuba@kernel.org> wrote: > On Thu, 10 Mar 2022 21:48:30 +0800 Mingbao Sun wrote: > > Since the kernel API 'kernel_setsockopt' was removed, and since the > > function ‘tcp_set_congestion_control’ is just the real underlying guy > > handling this job, so it makes sense to get it exported. > > Do you happen to have a reference to the commit which removed > kernel_setsockopt and the justification? My knee jerk reaction > would the that's a better path than allowing in-kernel socket users > to poke at internal functions even if that works as of today. FYI (Sorry for putting URLs in the mail): kernel_setsockopt disappeared from kernel v5.8, and all the relevant users have switched to dedicated small functions. The mail thread: https://lists.archive.carbon60.com/linux/kernel/3712394 The commit: https://github.com/torvalds/linux/commit/5a892ff2facb4548c17c05931ed899038a0da63e
On Thu, Mar 10, 2022 at 12:48:25PM -0800, Jakub Kicinski wrote: > On Thu, 10 Mar 2022 21:48:30 +0800 Mingbao Sun wrote: > > Since the kernel API 'kernel_setsockopt' was removed, and since the > > function ‘tcp_set_congestion_control’ is just the real underlying guy > > handling this job, so it makes sense to get it exported. > > Do you happen to have a reference to the commit which removed > kernel_setsockopt and the justification? My knee jerk reaction > would the that's a better path than allowing in-kernel socket users > to poke at internal functions even if that works as of today. This was part of the set_fs() removal. Back then we decided we'd rather have type-safe APIs for in-kernel users, which in total was a major removal of code lines.
On Fri, 11 Mar 2022 08:19:30 +0100 Christoph Hellwig wrote: > On Thu, Mar 10, 2022 at 12:48:25PM -0800, Jakub Kicinski wrote: > > On Thu, 10 Mar 2022 21:48:30 +0800 Mingbao Sun wrote: > > > Since the kernel API 'kernel_setsockopt' was removed, and since the > > > function ‘tcp_set_congestion_control’ is just the real underlying guy > > > handling this job, so it makes sense to get it exported. > > > > Do you happen to have a reference to the commit which removed > > kernel_setsockopt and the justification? My knee jerk reaction > > would the that's a better path than allowing in-kernel socket users > > to poke at internal functions even if that works as of today. > > This was part of the set_fs() removal. Back then we decided we'd rather > have type-safe APIs for in-kernel users, which in total was a major > removal of code lines. I see, thanks. I guess no point speculating, we can revisit if any bugs due to direct callers actually materialize.
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c index db5831e6c136..5d77f3e7278e 100644 --- a/net/ipv4/tcp_cong.c +++ b/net/ipv4/tcp_cong.c @@ -383,6 +383,7 @@ int tcp_set_congestion_control(struct sock *sk, const char *name, bool load, rcu_read_unlock(); return err; } +EXPORT_SYMBOL_GPL(tcp_set_congestion_control); /* Slow start is used when congestion window is no greater than the slow start * threshold. We base on RFC2581 and also handle stretch ACKs properly.