* [net PATCH] bpf: remove SK_REDIRECT from UAPI
@ 2017-11-01 2:17 John Fastabend
2017-11-01 2:44 ` David Miller
2017-11-01 8:26 ` Jiri Pirko
0 siblings, 2 replies; 5+ messages in thread
From: John Fastabend @ 2017-11-01 2:17 UTC (permalink / raw)
To: alexei.starovoitov, daniel, davem; +Cc: netdev
Now that SK_REDIRECT is no longer a valid return code. Remove it
from the UAPI completely. Then do a namespace remapping internal
to sockmap so SK_REDIRECT is no longer externally visible.
Patchs primary change is to do a namechange from SK_REDIRECT to
__SK_REDIRECT
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/uapi/linux/bpf.h | 1 -
kernel/bpf/sockmap.c | 16 ++++++++++++----
tools/include/uapi/linux/bpf.h | 3 +--
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0d7948c..7bf4c75 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -788,7 +788,6 @@ struct xdp_md {
enum sk_action {
SK_DROP = 0,
SK_PASS,
- SK_REDIRECT,
};
#define BPF_TAG_SIZE 8
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 66f00a2..dbd7b32 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -101,13 +101,19 @@ static inline void bpf_compute_data_end_sk_skb(struct sk_buff *skb)
TCP_SKB_CB(skb)->bpf.data_end = skb->data + skb_headlen(skb);
}
+enum __sk_action {
+ __SK_DROP = 0,
+ __SK_PASS,
+ __SK_REDIRECT,
+};
+
static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
{
struct bpf_prog *prog = READ_ONCE(psock->bpf_verdict);
int rc;
if (unlikely(!prog))
- return SK_DROP;
+ return __SK_DROP;
skb_orphan(skb);
/* We need to ensure that BPF metadata for maps is also cleared
@@ -122,8 +128,10 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
preempt_enable();
skb->sk = NULL;
+ /* Moving return codes from UAPI namespace into internal namespace */
return rc == SK_PASS ?
- (TCP_SKB_CB(skb)->bpf.map ? SK_REDIRECT : SK_PASS) : SK_DROP;
+ (TCP_SKB_CB(skb)->bpf.map ? __SK_REDIRECT : __SK_PASS) :
+ __SK_DROP;
}
static void smap_do_verdict(struct smap_psock *psock, struct sk_buff *skb)
@@ -133,7 +141,7 @@ static void smap_do_verdict(struct smap_psock *psock, struct sk_buff *skb)
rc = smap_verdict_func(psock, skb);
switch (rc) {
- case SK_REDIRECT:
+ case __SK_REDIRECT:
sk = do_sk_redirect_map(skb);
if (likely(sk)) {
struct smap_psock *peer = smap_psock_sk(sk);
@@ -149,7 +157,7 @@ static void smap_do_verdict(struct smap_psock *psock, struct sk_buff *skb)
}
}
/* Fall through and free skb otherwise */
- case SK_DROP:
+ case __SK_DROP:
default:
kfree_skb(skb);
}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index c174971..01cc7ba 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -576,7 +576,7 @@ enum bpf_attach_type {
* @map: pointer to sockmap
* @key: key to lookup sock in map
* @flags: reserved for future use
- * Return: SK_REDIRECT
+ * Return: SK_PASS
*
* int bpf_sock_map_update(skops, map, key, flags)
* @skops: pointer to bpf_sock_ops
@@ -789,7 +789,6 @@ struct xdp_md {
enum sk_action {
SK_DROP = 0,
SK_PASS,
- SK_REDIRECT,
};
#define BPF_TAG_SIZE 8
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [net PATCH] bpf: remove SK_REDIRECT from UAPI
2017-11-01 2:17 [net PATCH] bpf: remove SK_REDIRECT from UAPI John Fastabend
@ 2017-11-01 2:44 ` David Miller
2017-11-01 8:26 ` Jiri Pirko
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2017-11-01 2:44 UTC (permalink / raw)
To: john.fastabend; +Cc: alexei.starovoitov, daniel, netdev
From: John Fastabend <john.fastabend@gmail.com>
Date: Tue, 31 Oct 2017 19:17:31 -0700
> Now that SK_REDIRECT is no longer a valid return code. Remove it
> from the UAPI completely. Then do a namespace remapping internal
> to sockmap so SK_REDIRECT is no longer externally visible.
>
> Patchs primary change is to do a namechange from SK_REDIRECT to
> __SK_REDIRECT
>
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net PATCH] bpf: remove SK_REDIRECT from UAPI
2017-11-01 2:17 [net PATCH] bpf: remove SK_REDIRECT from UAPI John Fastabend
2017-11-01 2:44 ` David Miller
@ 2017-11-01 8:26 ` Jiri Pirko
2017-11-01 13:50 ` John Fastabend
1 sibling, 1 reply; 5+ messages in thread
From: Jiri Pirko @ 2017-11-01 8:26 UTC (permalink / raw)
To: John Fastabend; +Cc: alexei.starovoitov, daniel, davem, netdev
Wed, Nov 01, 2017 at 03:17:31AM CET, john.fastabend@gmail.com wrote:
>Now that SK_REDIRECT is no longer a valid return code. Remove it
>from the UAPI completely. Then do a namespace remapping internal
>to sockmap so SK_REDIRECT is no longer externally visible.
>
>Patchs primary change is to do a namechange from SK_REDIRECT to
>__SK_REDIRECT
>
>Reported-by: Alexei Starovoitov <ast@kernel.org>
>Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>---
> include/uapi/linux/bpf.h | 1 -
> kernel/bpf/sockmap.c | 16 ++++++++++++----
> tools/include/uapi/linux/bpf.h | 3 +--
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
>diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>index 0d7948c..7bf4c75 100644
>--- a/include/uapi/linux/bpf.h
>+++ b/include/uapi/linux/bpf.h
>@@ -788,7 +788,6 @@ struct xdp_md {
> enum sk_action {
> SK_DROP = 0,
> SK_PASS,
>- SK_REDIRECT,
Is it really ok to do uapi changes like this?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net PATCH] bpf: remove SK_REDIRECT from UAPI
2017-11-01 8:26 ` Jiri Pirko
@ 2017-11-01 13:50 ` John Fastabend
2017-11-01 13:57 ` Jiri Pirko
0 siblings, 1 reply; 5+ messages in thread
From: John Fastabend @ 2017-11-01 13:50 UTC (permalink / raw)
To: Jiri Pirko; +Cc: alexei.starovoitov, daniel, davem, netdev
On 11/01/2017 01:26 AM, Jiri Pirko wrote:
> Wed, Nov 01, 2017 at 03:17:31AM CET, john.fastabend@gmail.com wrote:
>> Now that SK_REDIRECT is no longer a valid return code. Remove it
>>from the UAPI completely. Then do a namespace remapping internal
>> to sockmap so SK_REDIRECT is no longer externally visible.
>>
>> Patchs primary change is to do a namechange from SK_REDIRECT to
>> __SK_REDIRECT
>>
>> Reported-by: Alexei Starovoitov <ast@kernel.org>
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> ---
>> include/uapi/linux/bpf.h | 1 -
>> kernel/bpf/sockmap.c | 16 ++++++++++++----
>> tools/include/uapi/linux/bpf.h | 3 +--
>> 3 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 0d7948c..7bf4c75 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -788,7 +788,6 @@ struct xdp_md {
>> enum sk_action {
>> SK_DROP = 0,
>> SK_PASS,
>> - SK_REDIRECT,
>
> Is it really ok to do uapi changes like this?
>
sockmap feature was only added in net so there is no released kernel
with SK_REDIRECT. And there is no user facing code that can interpret
the SK_REDIRECT return code it is only helpful for interface internals.
So best to remove it rather than have it enshrined in UAPI
unnecessarily.
.John
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net PATCH] bpf: remove SK_REDIRECT from UAPI
2017-11-01 13:50 ` John Fastabend
@ 2017-11-01 13:57 ` Jiri Pirko
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Pirko @ 2017-11-01 13:57 UTC (permalink / raw)
To: John Fastabend; +Cc: alexei.starovoitov, daniel, davem, netdev
Wed, Nov 01, 2017 at 02:50:29PM CET, john.fastabend@gmail.com wrote:
>On 11/01/2017 01:26 AM, Jiri Pirko wrote:
>> Wed, Nov 01, 2017 at 03:17:31AM CET, john.fastabend@gmail.com wrote:
>>> Now that SK_REDIRECT is no longer a valid return code. Remove it
>>>from the UAPI completely. Then do a namespace remapping internal
>>> to sockmap so SK_REDIRECT is no longer externally visible.
>>>
>>> Patchs primary change is to do a namechange from SK_REDIRECT to
>>> __SK_REDIRECT
>>>
>>> Reported-by: Alexei Starovoitov <ast@kernel.org>
>>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>>> ---
>>> include/uapi/linux/bpf.h | 1 -
>>> kernel/bpf/sockmap.c | 16 ++++++++++++----
>>> tools/include/uapi/linux/bpf.h | 3 +--
>>> 3 files changed, 13 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>>> index 0d7948c..7bf4c75 100644
>>> --- a/include/uapi/linux/bpf.h
>>> +++ b/include/uapi/linux/bpf.h
>>> @@ -788,7 +788,6 @@ struct xdp_md {
>>> enum sk_action {
>>> SK_DROP = 0,
>>> SK_PASS,
>>> - SK_REDIRECT,
>>
>> Is it really ok to do uapi changes like this?
>>
>
>sockmap feature was only added in net so there is no released kernel
>with SK_REDIRECT. And there is no user facing code that can interpret
>the SK_REDIRECT return code it is only helpful for interface internals.
>So best to remove it rather than have it enshrined in UAPI
>unnecessarily.
Okay. You should provide a "Fixes:" line that would make this clearer.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-01 13:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 2:17 [net PATCH] bpf: remove SK_REDIRECT from UAPI John Fastabend
2017-11-01 2:44 ` David Miller
2017-11-01 8:26 ` Jiri Pirko
2017-11-01 13:50 ` John Fastabend
2017-11-01 13:57 ` Jiri Pirko
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.