| Message ID | 20200730210728.2051-1-f.fainelli@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] tcp: Export tcp_write_queue_purge() | expand |
On Thu, Jul 30, 2020 at 2:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com> > --- > 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 <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com> >> --- >> 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.
From: Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com> wrote: > > On 7/30/20 2:16 PM, Eric Dumazet wrote: > > On Thu, Jul 30, 2020 at 2:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com> > >> --- > >> 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 <f.fainelli@gmail.com> wrote: >> >> On 7/30/20 2:16 PM, Eric Dumazet wrote: >>> On Thu, Jul 30, 2020 at 2:07 PM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com> >>>> --- >>>> 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 :)
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) {
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 <f.fainelli@gmail.com> --- net/ipv4/tcp.c | 1 + 1 file changed, 1 insertion(+)