If BPF_STREAM_PARSER is not set, gcc warns:
net/ipv4/tcp_bpf.c:483:12: warning: 'tcp_bpf_sendpage' defined but not used [-Wunused-function]
net/ipv4/tcp_bpf.c:395:12: warning: 'tcp_bpf_sendmsg' defined but not used [-Wunused-function]
net/ipv4/tcp_bpf.c:13:13: warning: 'tcp_bpf_stream_read' defined but not used [-Wunused-function]
Moves the unused functions into the #ifdef
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: YueHaibing <[email protected]>
---
net/ipv4/tcp_bpf.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index fe7b4fbc31c1..37c91f25cae3 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -10,19 +10,6 @@
#include <net/inet_common.h>
#include <net/tls.h>
-static bool tcp_bpf_stream_read(const struct sock *sk)
-{
- struct sk_psock *psock;
- bool empty = true;
-
- rcu_read_lock();
- psock = sk_psock(sk);
- if (likely(psock))
- empty = list_empty(&psock->ingress_msg);
- rcu_read_unlock();
- return !empty;
-}
-
static int tcp_bpf_wait_data(struct sock *sk, struct sk_psock *psock,
int flags, long timeo, int *err)
{
@@ -298,6 +285,20 @@ int tcp_bpf_sendmsg_redir(struct sock *sk, struct sk_msg *msg,
}
EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
+#ifdef CONFIG_BPF_STREAM_PARSER
+static bool tcp_bpf_stream_read(const struct sock *sk)
+{
+ struct sk_psock *psock;
+ bool empty = true;
+
+ rcu_read_lock();
+ psock = sk_psock(sk);
+ if (likely(psock))
+ empty = list_empty(&psock->ingress_msg);
+ rcu_read_unlock();
+ return !empty;
+}
+
static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
struct sk_msg *msg, int *copied, int flags)
{
@@ -528,7 +529,6 @@ static int tcp_bpf_sendpage(struct sock *sk, struct page *page, int offset,
return copied ? copied : err;
}
-#ifdef CONFIG_BPF_STREAM_PARSER
enum {
TCP_BPF_IPV4,
TCP_BPF_IPV6,
--
2.17.1
On Thu, 19 Mar 2020 at 12:47, YueHaibing <[email protected]> wrote:
>
> If BPF_STREAM_PARSER is not set, gcc warns:
>
> net/ipv4/tcp_bpf.c:483:12: warning: 'tcp_bpf_sendpage' defined but not used [-Wunused-function]
> net/ipv4/tcp_bpf.c:395:12: warning: 'tcp_bpf_sendmsg' defined but not used [-Wunused-function]
> net/ipv4/tcp_bpf.c:13:13: warning: 'tcp_bpf_stream_read' defined but not used [-Wunused-function]
>
> Moves the unused functions into the #ifdef
Thanks for fixing this.
Reviewed-by: Lorenz Bauer <[email protected]>
>
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: YueHaibing <[email protected]>
> ---
> net/ipv4/tcp_bpf.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index fe7b4fbc31c1..37c91f25cae3 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -10,19 +10,6 @@
> #include <net/inet_common.h>
> #include <net/tls.h>
>
> -static bool tcp_bpf_stream_read(const struct sock *sk)
> -{
> - struct sk_psock *psock;
> - bool empty = true;
> -
> - rcu_read_lock();
> - psock = sk_psock(sk);
> - if (likely(psock))
> - empty = list_empty(&psock->ingress_msg);
> - rcu_read_unlock();
> - return !empty;
> -}
> -
> static int tcp_bpf_wait_data(struct sock *sk, struct sk_psock *psock,
> int flags, long timeo, int *err)
> {
> @@ -298,6 +285,20 @@ int tcp_bpf_sendmsg_redir(struct sock *sk, struct sk_msg *msg,
> }
> EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
>
> +#ifdef CONFIG_BPF_STREAM_PARSER
> +static bool tcp_bpf_stream_read(const struct sock *sk)
> +{
> + struct sk_psock *psock;
> + bool empty = true;
> +
> + rcu_read_lock();
> + psock = sk_psock(sk);
> + if (likely(psock))
> + empty = list_empty(&psock->ingress_msg);
> + rcu_read_unlock();
> + return !empty;
> +}
> +
> static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
> struct sk_msg *msg, int *copied, int flags)
> {
> @@ -528,7 +529,6 @@ static int tcp_bpf_sendpage(struct sock *sk, struct page *page, int offset,
> return copied ? copied : err;
> }
>
> -#ifdef CONFIG_BPF_STREAM_PARSER
> enum {
> TCP_BPF_IPV4,
> TCP_BPF_IPV6,
> --
> 2.17.1
>
>
--
Lorenz Bauer | Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
http://www.cloudflare.com
On Thu, Mar 19, 2020 at 01:46 PM CET, YueHaibing wrote:
> If BPF_STREAM_PARSER is not set, gcc warns:
>
> net/ipv4/tcp_bpf.c:483:12: warning: 'tcp_bpf_sendpage' defined but not used [-Wunused-function]
> net/ipv4/tcp_bpf.c:395:12: warning: 'tcp_bpf_sendmsg' defined but not used [-Wunused-function]
> net/ipv4/tcp_bpf.c:13:13: warning: 'tcp_bpf_stream_read' defined but not used [-Wunused-function]
>
> Moves the unused functions into the #ifdef
>
> Reported-by: Hulk Robot <[email protected]>
> Signed-off-by: YueHaibing <[email protected]>
> ---
In addition to this fix, looks like tcp_bpf_recvmsg can be static and
also conditional on CONFIG_BPF_STREAM_PARSER.
Reviewed-by: Jakub Sitnicki <[email protected]>
On Thu, Mar 19, 2020 at 10:00 AM Jakub Sitnicki <[email protected]> wrote:
>
> On Thu, Mar 19, 2020 at 01:46 PM CET, YueHaibing wrote:
> > If BPF_STREAM_PARSER is not set, gcc warns:
> >
> > net/ipv4/tcp_bpf.c:483:12: warning: 'tcp_bpf_sendpage' defined but not used [-Wunused-function]
> > net/ipv4/tcp_bpf.c:395:12: warning: 'tcp_bpf_sendmsg' defined but not used [-Wunused-function]
> > net/ipv4/tcp_bpf.c:13:13: warning: 'tcp_bpf_stream_read' defined but not used [-Wunused-function]
> >
> > Moves the unused functions into the #ifdef
> >
> > Reported-by: Hulk Robot <[email protected]>
> > Signed-off-by: YueHaibing <[email protected]>
> > ---
>
> In addition to this fix, looks like tcp_bpf_recvmsg can be static and
> also conditional on CONFIG_BPF_STREAM_PARSER.
>
> Reviewed-by: Jakub Sitnicki <[email protected]>
Fixes tag is missing as well?
On 2020/3/20 1:00, Jakub Sitnicki wrote:
> On Thu, Mar 19, 2020 at 01:46 PM CET, YueHaibing wrote:
>> If BPF_STREAM_PARSER is not set, gcc warns:
>>
>> net/ipv4/tcp_bpf.c:483:12: warning: 'tcp_bpf_sendpage' defined but not used [-Wunused-function]
>> net/ipv4/tcp_bpf.c:395:12: warning: 'tcp_bpf_sendmsg' defined but not used [-Wunused-function]
>> net/ipv4/tcp_bpf.c:13:13: warning: 'tcp_bpf_stream_read' defined but not used [-Wunused-function]
>>
>> Moves the unused functions into the #ifdef
>>
>> Reported-by: Hulk Robot <[email protected]>
>> Signed-off-by: YueHaibing <[email protected]>
>> ---
>
> In addition to this fix, looks like tcp_bpf_recvmsg can be static and
> also conditional on CONFIG_BPF_STREAM_PARSER.
Thanks, will do this in next version.
>
> Reviewed-by: Jakub Sitnicki <[email protected]>
>
> .
>
Minor cleanups for tcp_bpf.c
YueHaibing (2):
bpf: tcp: Fix unused function warnings
bpf: tcp: Make tcp_bpf_recvmsg static
include/net/tcp.h | 2 -
net/ipv4/tcp_bpf.c | 152 ++++++++++++++++++++++-----------------------
2 files changed, 76 insertions(+), 78 deletions(-)
--
2.17.1
After commit f747632b608f ("bpf: sockmap: Move generic sockmap
hooks from BPF TCP"), tcp_bpf_recvmsg() is not used out of
tcp_bpf.c, so make it static and remove it from tcp.h. Also move
it to BPF_STREAM_PARSER #ifdef to fix unused function warnings.
Signed-off-by: YueHaibing <[email protected]>
---
include/net/tcp.h | 2 -
net/ipv4/tcp_bpf.c | 124 ++++++++++++++++++++++-----------------------
2 files changed, 62 insertions(+), 64 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 43fa07a36fa6..5fa9eacd965a 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2207,8 +2207,6 @@ static inline void tcp_bpf_clone(const struct sock *sk, struct sock *newsk)
#ifdef CONFIG_NET_SOCK_MSG
int tcp_bpf_sendmsg_redir(struct sock *sk, struct sk_msg *msg, u32 bytes,
int flags);
-int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
- int nonblock, int flags, int *addr_len);
int __tcp_bpf_recvmsg(struct sock *sk, struct sk_psock *psock,
struct msghdr *msg, int len, int flags);
#endif /* CONFIG_NET_SOCK_MSG */
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 37c91f25cae3..5a05327f97c1 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -10,25 +10,6 @@
#include <net/inet_common.h>
#include <net/tls.h>
-static int tcp_bpf_wait_data(struct sock *sk, struct sk_psock *psock,
- int flags, long timeo, int *err)
-{
- DEFINE_WAIT_FUNC(wait, woken_wake_function);
- int ret = 0;
-
- if (!timeo)
- return ret;
-
- add_wait_queue(sk_sleep(sk), &wait);
- sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
- ret = sk_wait_event(sk, &timeo,
- !list_empty(&psock->ingress_msg) ||
- !skb_queue_empty(&sk->sk_receive_queue), &wait);
- sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
- remove_wait_queue(sk_sleep(sk), &wait);
- return ret;
-}
-
int __tcp_bpf_recvmsg(struct sock *sk, struct sk_psock *psock,
struct msghdr *msg, int len, int flags)
{
@@ -102,49 +83,6 @@ int __tcp_bpf_recvmsg(struct sock *sk, struct sk_psock *psock,
}
EXPORT_SYMBOL_GPL(__tcp_bpf_recvmsg);
-int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
- int nonblock, int flags, int *addr_len)
-{
- struct sk_psock *psock;
- int copied, ret;
-
- psock = sk_psock_get(sk);
- if (unlikely(!psock))
- return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
- if (unlikely(flags & MSG_ERRQUEUE))
- return inet_recv_error(sk, msg, len, addr_len);
- if (!skb_queue_empty(&sk->sk_receive_queue) &&
- sk_psock_queue_empty(psock))
- return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
- lock_sock(sk);
-msg_bytes_ready:
- copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags);
- if (!copied) {
- int data, err = 0;
- long timeo;
-
- timeo = sock_rcvtimeo(sk, nonblock);
- data = tcp_bpf_wait_data(sk, psock, flags, timeo, &err);
- if (data) {
- if (!sk_psock_queue_empty(psock))
- goto msg_bytes_ready;
- release_sock(sk);
- sk_psock_put(sk, psock);
- return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
- }
- if (err) {
- ret = err;
- goto out;
- }
- copied = -EAGAIN;
- }
- ret = copied;
-out:
- release_sock(sk);
- sk_psock_put(sk, psock);
- return ret;
-}
-
static int bpf_tcp_ingress(struct sock *sk, struct sk_psock *psock,
struct sk_msg *msg, u32 apply_bytes, int flags)
{
@@ -299,6 +237,68 @@ static bool tcp_bpf_stream_read(const struct sock *sk)
return !empty;
}
+static int tcp_bpf_wait_data(struct sock *sk, struct sk_psock *psock,
+ int flags, long timeo, int *err)
+{
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ int ret = 0;
+
+ if (!timeo)
+ return ret;
+
+ add_wait_queue(sk_sleep(sk), &wait);
+ sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+ ret = sk_wait_event(sk, &timeo,
+ !list_empty(&psock->ingress_msg) ||
+ !skb_queue_empty(&sk->sk_receive_queue), &wait);
+ sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+ remove_wait_queue(sk_sleep(sk), &wait);
+ return ret;
+}
+
+static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
+ int nonblock, int flags, int *addr_len)
+{
+ struct sk_psock *psock;
+ int copied, ret;
+
+ psock = sk_psock_get(sk);
+ if (unlikely(!psock))
+ return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+ if (unlikely(flags & MSG_ERRQUEUE))
+ return inet_recv_error(sk, msg, len, addr_len);
+ if (!skb_queue_empty(&sk->sk_receive_queue) &&
+ sk_psock_queue_empty(psock))
+ return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+ lock_sock(sk);
+msg_bytes_ready:
+ copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags);
+ if (!copied) {
+ int data, err = 0;
+ long timeo;
+
+ timeo = sock_rcvtimeo(sk, nonblock);
+ data = tcp_bpf_wait_data(sk, psock, flags, timeo, &err);
+ if (data) {
+ if (!sk_psock_queue_empty(psock))
+ goto msg_bytes_ready;
+ release_sock(sk);
+ sk_psock_put(sk, psock);
+ return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+ }
+ if (err) {
+ ret = err;
+ goto out;
+ }
+ copied = -EAGAIN;
+ }
+ ret = copied;
+out:
+ release_sock(sk);
+ sk_psock_put(sk, psock);
+ return ret;
+}
+
static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
struct sk_msg *msg, int *copied, int flags)
{
--
2.17.1
If BPF_STREAM_PARSER is not set, gcc warns:
net/ipv4/tcp_bpf.c:483:12: warning: 'tcp_bpf_sendpage' defined but not used [-Wunused-function]
net/ipv4/tcp_bpf.c:395:12: warning: 'tcp_bpf_sendmsg' defined but not used [-Wunused-function]
net/ipv4/tcp_bpf.c:13:13: warning: 'tcp_bpf_stream_read' defined but not used [-Wunused-function]
Moves the unused functions into the #ifdef
Reported-by: Hulk Robot <[email protected]>
Fixes: f747632b608f ("bpf: sockmap: Move generic sockmap hooks from BPF TCP")
Signed-off-by: YueHaibing <[email protected]>
Reviewed-by: Lorenz Bauer <[email protected]>
Reviewed-by: Jakub Sitnicki <[email protected]>
---
net/ipv4/tcp_bpf.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index fe7b4fbc31c1..37c91f25cae3 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -10,19 +10,6 @@
#include <net/inet_common.h>
#include <net/tls.h>
-static bool tcp_bpf_stream_read(const struct sock *sk)
-{
- struct sk_psock *psock;
- bool empty = true;
-
- rcu_read_lock();
- psock = sk_psock(sk);
- if (likely(psock))
- empty = list_empty(&psock->ingress_msg);
- rcu_read_unlock();
- return !empty;
-}
-
static int tcp_bpf_wait_data(struct sock *sk, struct sk_psock *psock,
int flags, long timeo, int *err)
{
@@ -298,6 +285,20 @@ int tcp_bpf_sendmsg_redir(struct sock *sk, struct sk_msg *msg,
}
EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
+#ifdef CONFIG_BPF_STREAM_PARSER
+static bool tcp_bpf_stream_read(const struct sock *sk)
+{
+ struct sk_psock *psock;
+ bool empty = true;
+
+ rcu_read_lock();
+ psock = sk_psock(sk);
+ if (likely(psock))
+ empty = list_empty(&psock->ingress_msg);
+ rcu_read_unlock();
+ return !empty;
+}
+
static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
struct sk_msg *msg, int *copied, int flags)
{
@@ -528,7 +529,6 @@ static int tcp_bpf_sendpage(struct sock *sk, struct page *page, int offset,
return copied ? copied : err;
}
-#ifdef CONFIG_BPF_STREAM_PARSER
enum {
TCP_BPF_IPV4,
TCP_BPF_IPV6,
--
2.17.1
On 3/19/20 7:34 PM, YueHaibing wrote:
> If BPF_STREAM_PARSER is not set, gcc warns:
>
> net/ipv4/tcp_bpf.c:483:12: warning: 'tcp_bpf_sendpage' defined but not used [-Wunused-function]
> net/ipv4/tcp_bpf.c:395:12: warning: 'tcp_bpf_sendmsg' defined but not used [-Wunused-function]
> net/ipv4/tcp_bpf.c:13:13: warning: 'tcp_bpf_stream_read' defined but not used [-Wunused-function]
>
> Moves the unused functions into the #ifdef
Maybe explicit "into the #ifdef CONFIG_BPF_STREAM_PARSER"?
>
> Reported-by: Hulk Robot <[email protected]>
> Fixes: f747632b608f ("bpf: sockmap: Move generic sockmap hooks from BPF TCP")
> Signed-off-by: YueHaibing <[email protected]>
> Reviewed-by: Lorenz Bauer <[email protected]>
> Reviewed-by: Jakub Sitnicki <[email protected]>
Acked-by: Yonghong Song <[email protected]>
On 3/19/20 7:34 PM, YueHaibing wrote:
> After commit f747632b608f ("bpf: sockmap: Move generic sockmap
> hooks from BPF TCP"), tcp_bpf_recvmsg() is not used out of
> tcp_bpf.c, so make it static and remove it from tcp.h. Also move
> it to BPF_STREAM_PARSER #ifdef to fix unused function warnings.
>
Fixes: f747632b608f ("bpf: sockmap: Move generic sockmap hooks from BPF
TCP")
> Signed-off-by: YueHaibing <[email protected]>
> ---
> include/net/tcp.h | 2 -
> net/ipv4/tcp_bpf.c | 124 ++++++++++++++++++++++-----------------------
> 2 files changed, 62 insertions(+), 64 deletions(-)
Other than the above fixes,
Acked-by: Yonghong Song <[email protected]>
On 3/20/20 5:21 AM, Yonghong Song wrote:
> On 3/19/20 7:34 PM, YueHaibing wrote:
>> If BPF_STREAM_PARSER is not set, gcc warns:
>>
>> net/ipv4/tcp_bpf.c:483:12: warning: 'tcp_bpf_sendpage' defined but not used [-Wunused-function]
>> net/ipv4/tcp_bpf.c:395:12: warning: 'tcp_bpf_sendmsg' defined but not used [-Wunused-function]
>> net/ipv4/tcp_bpf.c:13:13: warning: 'tcp_bpf_stream_read' defined but not used [-Wunused-function]
>>
>> Moves the unused functions into the #ifdef
>
> Maybe explicit "into the #ifdef CONFIG_BPF_STREAM_PARSER"?
>
>> Reported-by: Hulk Robot <[email protected]>
>> Fixes: f747632b608f ("bpf: sockmap: Move generic sockmap hooks from BPF TCP")
>> Signed-off-by: YueHaibing <[email protected]>
>> Reviewed-by: Lorenz Bauer <[email protected]>
>> Reviewed-by: Jakub Sitnicki <[email protected]>
>
> Acked-by: Yonghong Song <[email protected]>
Both applied and addressed feedback from Yonghong, thanks!