After tcp_write_queue_purge() got uninlined with commit ac3f09ba3e49
("tcp: uninline tcp_write_queue_purge()"), it became no longer possible
to reference this symbol from kernel modules.
Fixes: ac3f09ba3e49 ("tcp: uninline tcp_write_queue_purge()")
Signed-off-by: Florian Fainelli <[email protected]>
---
net/ipv4/tcp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 6f0caf9a866d..ea9d296a8380 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2626,6 +2626,7 @@ void tcp_write_queue_purge(struct sock *sk)
tcp_sk(sk)->packets_out = 0;
inet_csk(sk)->icsk_backoff = 0;
}
+EXPORT_SYMBOL(tcp_write_queue_purge);
int tcp_disconnect(struct sock *sk, int flags)
{
--
2.17.1
On Thu, Jul 30, 2020 at 2:07 PM Florian Fainelli <[email protected]> wrote:
>
> After tcp_write_queue_purge() got uninlined with commit ac3f09ba3e49
> ("tcp: uninline tcp_write_queue_purge()"), it became no longer possible
> to reference this symbol from kernel modules.
>
> Fixes: ac3f09ba3e49 ("tcp: uninline tcp_write_queue_purge()")
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> net/ipv4/tcp.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 6f0caf9a866d..ea9d296a8380 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2626,6 +2626,7 @@ void tcp_write_queue_purge(struct sock *sk)
> tcp_sk(sk)->packets_out = 0;
> inet_csk(sk)->icsk_backoff = 0;
> }
> +EXPORT_SYMBOL(tcp_write_queue_purge);
>
> int tcp_disconnect(struct sock *sk, int flags)
> {
> --
> 2.17.1
>
Hmmm.... which module would need this exactly ?
How come it took 3 years to discover this issue ?
On 7/30/20 2:16 PM, Eric Dumazet wrote:
> On Thu, Jul 30, 2020 at 2:07 PM Florian Fainelli <[email protected]> wrote:
>>
>> After tcp_write_queue_purge() got uninlined with commit ac3f09ba3e49
>> ("tcp: uninline tcp_write_queue_purge()"), it became no longer possible
>> to reference this symbol from kernel modules.
>>
>> Fixes: ac3f09ba3e49 ("tcp: uninline tcp_write_queue_purge()")
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> net/ipv4/tcp.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index 6f0caf9a866d..ea9d296a8380 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -2626,6 +2626,7 @@ void tcp_write_queue_purge(struct sock *sk)
>> tcp_sk(sk)->packets_out = 0;
>> inet_csk(sk)->icsk_backoff = 0;
>> }
>> +EXPORT_SYMBOL(tcp_write_queue_purge);
>>
>> int tcp_disconnect(struct sock *sk, int flags)
>> {
>> --
>> 2.17.1
>>
>
> Hmmm.... which module would need this exactly ?
None in tree unfortunately, and I doubt it would be published one day.
For consistency one could argue that given it used to be accessible, and
other symbols within net/ipv4/tcp.c are also exported, so this should
one be. Not going to hold that line of argumentation more than in this
email, if you object to it, that would be completely fine with me.
>
> How come it took 3 years to discover this issue ?
We just upgraded our downstream kernel from 4.9 to 5.4 and this is why
it took so long.
--
Florian
From: Florian Fainelli <[email protected]>
Date: Thu, 30 Jul 2020 14:23:50 -0700
> On 7/30/20 2:16 PM, Eric Dumazet wrote:
>> On Thu, Jul 30, 2020 at 2:07 PM Florian Fainelli <[email protected]> wrote:
>> Hmmm.... which module would need this exactly ?
>
> None in tree unfortunately, and I doubt it would be published one day.
> For consistency one could argue that given it used to be accessible, and
> other symbols within net/ipv4/tcp.c are also exported, so this should
> one be. Not going to hold that line of argumentation more than in this
> email, if you object to it, that would be completely fine with me.
>
>>
>> How come it took 3 years to discover this issue ?
>
> We just upgraded our downstream kernel from 4.9 to 5.4 and this is why
> it took so long.
We really can't do this, sorry.
On Thu, Jul 30, 2020 at 2:24 PM Florian Fainelli <[email protected]> wrote:
>
> On 7/30/20 2:16 PM, Eric Dumazet wrote:
> > On Thu, Jul 30, 2020 at 2:07 PM Florian Fainelli <[email protected]> wrote:
> >>
> >> After tcp_write_queue_purge() got uninlined with commit ac3f09ba3e49
> >> ("tcp: uninline tcp_write_queue_purge()"), it became no longer possible
> >> to reference this symbol from kernel modules.
> >>
> >> Fixes: ac3f09ba3e49 ("tcp: uninline tcp_write_queue_purge()")
> >> Signed-off-by: Florian Fainelli <[email protected]>
> >> ---
> >> net/ipv4/tcp.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> >> index 6f0caf9a866d..ea9d296a8380 100644
> >> --- a/net/ipv4/tcp.c
> >> +++ b/net/ipv4/tcp.c
> >> @@ -2626,6 +2626,7 @@ void tcp_write_queue_purge(struct sock *sk)
> >> tcp_sk(sk)->packets_out = 0;
> >> inet_csk(sk)->icsk_backoff = 0;
> >> }
> >> +EXPORT_SYMBOL(tcp_write_queue_purge);
> >>
> >> int tcp_disconnect(struct sock *sk, int flags)
> >> {
> >> --
> >> 2.17.1
> >>
> >
> > Hmmm.... which module would need this exactly ?
>
> None in tree unfortunately, and I doubt it would be published one day.
> For consistency one could argue that given it used to be accessible, and
> other symbols within net/ipv4/tcp.c are also exported, so this should
> one be. Not going to hold that line of argumentation more than in this
> email, if you object to it, that would be completely fine with me.
:)
>
> >
> > How come it took 3 years to discover this issue ?
>
> We just upgraded our downstream kernel from 4.9 to 5.4 and this is why
> it took so long.
It is not because TCP used an inline function in the past that it
means we have to keep
the equivalent function available for all possible out-of-tree modules.
Sorry, we can not accept that out-of-tree modules use TCP stack like that.
You will have to carry this change locally. Or even better get rid of it.
On 7/30/20 2:32 PM, Eric Dumazet wrote:
> On Thu, Jul 30, 2020 at 2:24 PM Florian Fainelli <[email protected]> wrote:
>>
>> On 7/30/20 2:16 PM, Eric Dumazet wrote:
>>> On Thu, Jul 30, 2020 at 2:07 PM Florian Fainelli <[email protected]> wrote:
>>>>
>>>> After tcp_write_queue_purge() got uninlined with commit ac3f09ba3e49
>>>> ("tcp: uninline tcp_write_queue_purge()"), it became no longer possible
>>>> to reference this symbol from kernel modules.
>>>>
>>>> Fixes: ac3f09ba3e49 ("tcp: uninline tcp_write_queue_purge()")
>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>> ---
>>>> net/ipv4/tcp.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>>> index 6f0caf9a866d..ea9d296a8380 100644
>>>> --- a/net/ipv4/tcp.c
>>>> +++ b/net/ipv4/tcp.c
>>>> @@ -2626,6 +2626,7 @@ void tcp_write_queue_purge(struct sock *sk)
>>>> tcp_sk(sk)->packets_out = 0;
>>>> inet_csk(sk)->icsk_backoff = 0;
>>>> }
>>>> +EXPORT_SYMBOL(tcp_write_queue_purge);
>>>>
>>>> int tcp_disconnect(struct sock *sk, int flags)
>>>> {
>>>> --
>>>> 2.17.1
>>>>
>>>
>>> Hmmm.... which module would need this exactly ?
>>
>> None in tree unfortunately, and I doubt it would be published one day.
>> For consistency one could argue that given it used to be accessible, and
>> other symbols within net/ipv4/tcp.c are also exported, so this should
>> one be. Not going to hold that line of argumentation more than in this
>> email, if you object to it, that would be completely fine with me.
>
> :)
>
>>
>>>
>>> How come it took 3 years to discover this issue ?
>>
>> We just upgraded our downstream kernel from 4.9 to 5.4 and this is why
>> it took so long.
>
> It is not because TCP used an inline function in the past that it
> means we have to keep
> the equivalent function available for all possible out-of-tree modules.
>
> Sorry, we can not accept that out-of-tree modules use TCP stack like that.
>
> You will have to carry this change locally. Or even better get rid of it.
Sure, that is completely fair, I had to try though :)
--
Florian