diff mbox series

[bpf-next,v5,2/2] bpf, xdp: restructure redirect actions

Message ID 20210227122139.183284-3-bjorn.topel@gmail.com
State New
Headers show
Series Optimize bpf_redirect_map()/xdp_do_redirect() | expand

Commit Message

Björn Töpel Feb. 27, 2021, 12:21 p.m. UTC 
From: Björn Töpel <bjorn.topel@intel.com>

The XDP_REDIRECT implementations for maps and non-maps are fairly
similar, but obviously need to take different code paths depending on
if the target is using a map or not. Today, the redirect targets for
XDP either uses a map, or is based on ifindex.

Here, an explicit redirect type is added to bpf_redirect_info, instead
of the actual map. Redirect type, map item/ifindex, and the map_id (if
any) is passed to xdp_do_redirect().

In addition to making the code easier to follow, using an explicit
type in bpf_redirect_info has a slight positive performance impact by
avoiding a pointer indirection for the map type lookup, and instead
use the cacheline for bpf_redirect_info.

Since the actual map is not passed via bpf_redirect_info anymore, the
map lookup is only done in the BPF helper. This means that the
bpf_clear_redirect_map() function can be removed. The actual map item
is RCU protected.

The bpf_redirect_info flags member is not used by XDP, and not
read/written any more. The map member is only written to when
required/used, and not unconditionally.

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 include/linux/filter.h     |  20 ++++--
 include/trace/events/xdp.h |  66 ++++++++++-------
 kernel/bpf/cpumap.c        |   4 +-
 kernel/bpf/devmap.c        |   7 +-
 net/core/filter.c          | 144 +++++++++++++++----------------------
 net/xdp/xskmap.c           |   4 +-
 6 files changed, 121 insertions(+), 124 deletions(-)

Comments

Daniel Borkmann March 5, 2021, 3:44 p.m. UTC | #1 
On 2/27/21 1:21 PM, Björn Töpel wrote:
[...]
> diff --git a/include/linux/filter.h b/include/linux/filter.h

> index 008691fd3b58..a7752badc2ec 100644

> --- a/include/linux/filter.h

> +++ b/include/linux/filter.h

> @@ -646,11 +646,20 @@ struct bpf_redirect_info {

>   	u32 flags;

>   	u32 tgt_index;

>   	void *tgt_value;

> -	struct bpf_map *map;

> +	u32 map_id;

> +	u32 tgt_type;

>   	u32 kern_flags;

>   	struct bpf_nh_params nh;

>   };

>   

> +enum xdp_redirect_type {

> +	XDP_REDIR_UNSET,

> +	XDP_REDIR_DEV_IFINDEX,


[...]

> +	XDP_REDIR_DEV_MAP,

> +	XDP_REDIR_CPU_MAP,

> +	XDP_REDIR_XSK_MAP,


Did you eval whether for these maps we can avoid the redundant def above by just
passing in map->map_type as ri->tgt_type and inferring the XDP_REDIR_UNSET from
invalid map_id of 0 (given the idr will never allocate such)?

[...]
> @@ -4068,10 +4039,9 @@ BPF_CALL_2(bpf_xdp_redirect, u32, ifindex, u64, flags)

>   	if (unlikely(flags))

>   		return XDP_ABORTED;

>   

> -	ri->flags = flags;

> -	ri->tgt_index = ifindex;

> -	ri->tgt_value = NULL;

> -	WRITE_ONCE(ri->map, NULL);

> +	ri->tgt_type = XDP_REDIR_DEV_IFINDEX;

> +	ri->tgt_index = 0;

> +	ri->tgt_value = (void *)(long)ifindex;


nit: Bit ugly to pass this in /read out this way, maybe union if we cannot use
tgt_index?

>   	return XDP_REDIRECT;

>   }

> diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c

> index 711acb3636b3..2c58d88aa69d 100644

> --- a/net/xdp/xskmap.c

> +++ b/net/xdp/xskmap.c

> @@ -87,7 +87,6 @@ static void xsk_map_free(struct bpf_map *map)

>   {

>   	struct xsk_map *m = container_of(map, struct xsk_map, map);

>   

> -	bpf_clear_redirect_map(map);

>   	synchronize_net();

>   	bpf_map_area_free(m);

>   }

> @@ -229,7 +228,8 @@ static int xsk_map_delete_elem(struct bpf_map *map, void *key)

>   

>   static int xsk_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)

>   {

> -	return __bpf_xdp_redirect_map(map, ifindex, flags, __xsk_map_lookup_elem);

> +	return __bpf_xdp_redirect_map(map, ifindex, flags, __xsk_map_lookup_elem,

> +				      XDP_REDIR_XSK_MAP);

>   }

>   

>   void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,

>
Björn Töpel March 5, 2021, 5:11 p.m. UTC | #2 
On 2021-03-05 16:44, Daniel Borkmann wrote:
> On 2/27/21 1:21 PM, Björn Töpel wrote:

> [...]

>> diff --git a/include/linux/filter.h b/include/linux/filter.h

>> index 008691fd3b58..a7752badc2ec 100644

>> --- a/include/linux/filter.h

>> +++ b/include/linux/filter.h

>> @@ -646,11 +646,20 @@ struct bpf_redirect_info {

>>       u32 flags;

>>       u32 tgt_index;

>>       void *tgt_value;

>> -    struct bpf_map *map;

>> +    u32 map_id;

>> +    u32 tgt_type;

>>       u32 kern_flags;

>>       struct bpf_nh_params nh;

>>   };

>> +enum xdp_redirect_type {

>> +    XDP_REDIR_UNSET,

>> +    XDP_REDIR_DEV_IFINDEX,

> 

> [...]

> 

>> +    XDP_REDIR_DEV_MAP,

>> +    XDP_REDIR_CPU_MAP,

>> +    XDP_REDIR_XSK_MAP,

> 

> Did you eval whether for these maps we can avoid the redundant def above 

> by just

> passing in map->map_type as ri->tgt_type and inferring the 

> XDP_REDIR_UNSET from

> invalid map_id of 0 (given the idr will never allocate such)?

>


I'll take a stab at it!


> [...]

>> @@ -4068,10 +4039,9 @@ BPF_CALL_2(bpf_xdp_redirect, u32, ifindex, u64, 

>> flags)

>>       if (unlikely(flags))

>>           return XDP_ABORTED;

>> -    ri->flags = flags;

>> -    ri->tgt_index = ifindex;

>> -    ri->tgt_value = NULL;

>> -    WRITE_ONCE(ri->map, NULL);

>> +    ri->tgt_type = XDP_REDIR_DEV_IFINDEX;

>> +    ri->tgt_index = 0;

>> +    ri->tgt_value = (void *)(long)ifindex;

> 

> nit: Bit ugly to pass this in /read out this way, maybe union if we 

> cannot use

> tgt_index?

>


Dito!


Thanks for the input! I'll get back with a v6!


Björn


>>       return XDP_REDIRECT;

>>   }

>> diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c

>> index 711acb3636b3..2c58d88aa69d 100644

>> --- a/net/xdp/xskmap.c

>> +++ b/net/xdp/xskmap.c

>> @@ -87,7 +87,6 @@ static void xsk_map_free(struct bpf_map *map)

>>   {

>>       struct xsk_map *m = container_of(map, struct xsk_map, map);

>> -    bpf_clear_redirect_map(map);

>>       synchronize_net();

>>       bpf_map_area_free(m);

>>   }

>> @@ -229,7 +228,8 @@ static int xsk_map_delete_elem(struct bpf_map 

>> *map, void *key)

>>   static int xsk_map_redirect(struct bpf_map *map, u32 ifindex, u64 

>> flags)

>>   {

>> -    return __bpf_xdp_redirect_map(map, ifindex, flags, 

>> __xsk_map_lookup_elem);

>> +    return __bpf_xdp_redirect_map(map, ifindex, flags, 

>> __xsk_map_lookup_elem,

>> +                      XDP_REDIR_XSK_MAP);

>>   }

>>   void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,

>>

>
Daniel Borkmann March 5, 2021, 10:56 p.m. UTC | #3 
On 3/5/21 6:11 PM, Björn Töpel wrote:
> On 2021-03-05 16:44, Daniel Borkmann wrote:

>> On 2/27/21 1:21 PM, Björn Töpel wrote:

>> [...]

>>> diff --git a/include/linux/filter.h b/include/linux/filter.h

>>> index 008691fd3b58..a7752badc2ec 100644

>>> --- a/include/linux/filter.h

>>> +++ b/include/linux/filter.h

>>> @@ -646,11 +646,20 @@ struct bpf_redirect_info {

>>>       u32 flags;

>>>       u32 tgt_index;

>>>       void *tgt_value;

>>> -    struct bpf_map *map;

>>> +    u32 map_id;

>>> +    u32 tgt_type;

>>>       u32 kern_flags;

>>>       struct bpf_nh_params nh;

>>>   };

>>> +enum xdp_redirect_type {

>>> +    XDP_REDIR_UNSET,

>>> +    XDP_REDIR_DEV_IFINDEX,

>>

>> [...]

>>

>>> +    XDP_REDIR_DEV_MAP,

>>> +    XDP_REDIR_CPU_MAP,

>>> +    XDP_REDIR_XSK_MAP,

>>

>> Did you eval whether for these maps we can avoid the redundant def above by just

>> passing in map->map_type as ri->tgt_type and inferring the XDP_REDIR_UNSET from

>> invalid map_id of 0 (given the idr will never allocate such)?

>>

> 

> I'll take a stab at it!


Sounds good, thanks! If it doesn't simplify or gets worse, we can always stick to
the one here.

>> [...]

>>> @@ -4068,10 +4039,9 @@ BPF_CALL_2(bpf_xdp_redirect, u32, ifindex, u64, flags)

>>>       if (unlikely(flags))

>>>           return XDP_ABORTED;

>>> -    ri->flags = flags;

>>> -    ri->tgt_index = ifindex;

>>> -    ri->tgt_value = NULL;

>>> -    WRITE_ONCE(ri->map, NULL);

>>> +    ri->tgt_type = XDP_REDIR_DEV_IFINDEX;

>>> +    ri->tgt_index = 0;

>>> +    ri->tgt_value = (void *)(long)ifindex;

>>

>> nit: Bit ugly to pass this in /read out this way, maybe union if we cannot use

>> tgt_index?

>>

> 

> Dito!

> 

> 

> Thanks for the input! I'll get back with a v6!

> 

> 

> Björn

> 

> 

>>>       return XDP_REDIRECT;

>>>   }

>>> diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c

>>> index 711acb3636b3..2c58d88aa69d 100644

>>> --- a/net/xdp/xskmap.c

>>> +++ b/net/xdp/xskmap.c

>>> @@ -87,7 +87,6 @@ static void xsk_map_free(struct bpf_map *map)

>>>   {

>>>       struct xsk_map *m = container_of(map, struct xsk_map, map);

>>> -    bpf_clear_redirect_map(map);

>>>       synchronize_net();

>>>       bpf_map_area_free(m);

>>>   }

>>> @@ -229,7 +228,8 @@ static int xsk_map_delete_elem(struct bpf_map *map, void *key)

>>>   static int xsk_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)

>>>   {

>>> -    return __bpf_xdp_redirect_map(map, ifindex, flags, __xsk_map_lookup_elem);

>>> +    return __bpf_xdp_redirect_map(map, ifindex, flags, __xsk_map_lookup_elem,

>>> +                      XDP_REDIR_XSK_MAP);

>>>   }

>>>   void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,

>>>

>>
diff mbox series

Patch

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 008691fd3b58..a7752badc2ec 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -646,11 +646,20 @@  struct bpf_redirect_info {
 	u32 flags;
 	u32 tgt_index;
 	void *tgt_value;
-	struct bpf_map *map;
+	u32 map_id;
+	u32 tgt_type;
 	u32 kern_flags;
 	struct bpf_nh_params nh;
 };
 
+enum xdp_redirect_type {
+	XDP_REDIR_UNSET,
+	XDP_REDIR_DEV_IFINDEX,
+	XDP_REDIR_DEV_MAP,
+	XDP_REDIR_CPU_MAP,
+	XDP_REDIR_XSK_MAP,
+};
+
 DECLARE_PER_CPU(struct bpf_redirect_info, bpf_redirect_info);
 
 /* flags for bpf_redirect_info kern_flags */
@@ -1473,7 +1482,8 @@  static inline bool bpf_sk_lookup_run_v6(struct net *net, int protocol,
 #endif /* IS_ENABLED(CONFIG_IPV6) */
 
 static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u32 ifindex, u64 flags,
-						  void *lookup_elem(struct bpf_map *map, u32 key))
+						  void *lookup_elem(struct bpf_map *map, u32 key),
+						  enum xdp_redirect_type type)
 {
 	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 
@@ -1488,13 +1498,13 @@  static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u32 ifind
 		 * performs multiple lookups, the last one always takes
 		 * precedence.
 		 */
-		WRITE_ONCE(ri->map, NULL);
+		ri->tgt_type = XDP_REDIR_UNSET;
 		return flags;
 	}
 
-	ri->flags = flags;
 	ri->tgt_index = ifindex;
-	WRITE_ONCE(ri->map, map);
+	ri->tgt_type = type;
+	ri->map_id = map->id;
 
 	return XDP_REDIRECT;
 }
diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index 76a97176ab81..538321735447 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -86,19 +86,15 @@  struct _bpf_dtab_netdev {
 };
 #endif /* __DEVMAP_OBJ_TYPE */
 
-#define devmap_ifindex(tgt, map)				\
-	(((map->map_type == BPF_MAP_TYPE_DEVMAP ||	\
-		  map->map_type == BPF_MAP_TYPE_DEVMAP_HASH)) ? \
-	  ((struct _bpf_dtab_netdev *)tgt)->dev->ifindex : 0)
-
 DECLARE_EVENT_CLASS(xdp_redirect_template,
 
 	TP_PROTO(const struct net_device *dev,
 		 const struct bpf_prog *xdp,
 		 const void *tgt, int err,
-		 const struct bpf_map *map, u32 index),
+		 enum xdp_redirect_type type,
+		 const struct bpf_redirect_info *ri),
 
-	TP_ARGS(dev, xdp, tgt, err, map, index),
+	TP_ARGS(dev, xdp, tgt, err, type, ri),
 
 	TP_STRUCT__entry(
 		__field(int, prog_id)
@@ -111,14 +107,30 @@  DECLARE_EVENT_CLASS(xdp_redirect_template,
 	),
 
 	TP_fast_assign(
+		u32 ifindex = 0, map_id = 0, index = ri->tgt_index;
+
+		switch (type) {
+		case XDP_REDIR_DEV_MAP:
+			ifindex = ((struct _bpf_dtab_netdev *)tgt)->dev->ifindex;
+			fallthrough;
+		case XDP_REDIR_CPU_MAP:
+		case XDP_REDIR_XSK_MAP:
+			map_id = ri->map_id;
+			break;
+		case XDP_REDIR_DEV_IFINDEX:
+			ifindex = (u32)(long)tgt;
+			break;
+		default:
+			break;
+		}
+
 		__entry->prog_id	= xdp->aux->id;
 		__entry->act		= XDP_REDIRECT;
 		__entry->ifindex	= dev->ifindex;
 		__entry->err		= err;
-		__entry->to_ifindex	= map ? devmap_ifindex(tgt, map) :
-						index;
-		__entry->map_id		= map ? map->id : 0;
-		__entry->map_index	= map ? index : 0;
+		__entry->to_ifindex	= ifindex;
+		__entry->map_id		= map_id;
+		__entry->map_index	= index;
 	),
 
 	TP_printk("prog_id=%d action=%s ifindex=%d to_ifindex=%d err=%d"
@@ -133,45 +145,49 @@  DEFINE_EVENT(xdp_redirect_template, xdp_redirect,
 	TP_PROTO(const struct net_device *dev,
 		 const struct bpf_prog *xdp,
 		 const void *tgt, int err,
-		 const struct bpf_map *map, u32 index),
-	TP_ARGS(dev, xdp, tgt, err, map, index)
+		 enum xdp_redirect_type type,
+		 const struct bpf_redirect_info *ri),
+	TP_ARGS(dev, xdp, tgt, err, type, ri)
 );
 
 DEFINE_EVENT(xdp_redirect_template, xdp_redirect_err,
 	TP_PROTO(const struct net_device *dev,
 		 const struct bpf_prog *xdp,
 		 const void *tgt, int err,
-		 const struct bpf_map *map, u32 index),
-	TP_ARGS(dev, xdp, tgt, err, map, index)
+		 enum xdp_redirect_type type,
+		 const struct bpf_redirect_info *ri),
+	TP_ARGS(dev, xdp, tgt, err, type, ri)
 );
 
 #define _trace_xdp_redirect(dev, xdp, to)				\
-	 trace_xdp_redirect(dev, xdp, NULL, 0, NULL, to)
+	trace_xdp_redirect(dev, xdp, NULL, 0, XDP_REDIR_DEV_IFINDEX, NULL)
 
 #define _trace_xdp_redirect_err(dev, xdp, to, err)			\
-	 trace_xdp_redirect_err(dev, xdp, NULL, err, NULL, to)
+	trace_xdp_redirect_err(dev, xdp, NULL, err, XDP_REDIR_DEV_IFINDEX, NULL)
 
-#define _trace_xdp_redirect_map(dev, xdp, to, map, index)		\
-	 trace_xdp_redirect(dev, xdp, to, 0, map, index)
+#define _trace_xdp_redirect_map(dev, xdp, to, type, ri)		\
+	trace_xdp_redirect(dev, xdp, to, 0, type, ri)
 
-#define _trace_xdp_redirect_map_err(dev, xdp, to, map, index, err)	\
-	 trace_xdp_redirect_err(dev, xdp, to, err, map, index)
+#define _trace_xdp_redirect_map_err(dev, xdp, to, type, ri, err)	\
+	trace_xdp_redirect_err(dev, xdp, to, err, type, ri)
 
 /* not used anymore, but kept around so as not to break old programs */
 DEFINE_EVENT(xdp_redirect_template, xdp_redirect_map,
 	TP_PROTO(const struct net_device *dev,
 		 const struct bpf_prog *xdp,
 		 const void *tgt, int err,
-		 const struct bpf_map *map, u32 index),
-	TP_ARGS(dev, xdp, tgt, err, map, index)
+		 enum xdp_redirect_type type,
+		 const struct bpf_redirect_info *ri),
+	TP_ARGS(dev, xdp, tgt, err, type, ri)
 );
 
 DEFINE_EVENT(xdp_redirect_template, xdp_redirect_map_err,
 	TP_PROTO(const struct net_device *dev,
 		 const struct bpf_prog *xdp,
 		 const void *tgt, int err,
-		 const struct bpf_map *map, u32 index),
-	TP_ARGS(dev, xdp, tgt, err, map, index)
+		 enum xdp_redirect_type type,
+		 const struct bpf_redirect_info *ri),
+	TP_ARGS(dev, xdp, tgt, err, type, ri)
 );
 
 TRACE_EVENT(xdp_cpumap_kthread,
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 7352d4160b7f..01b333e594d0 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -543,7 +543,6 @@  static void cpu_map_free(struct bpf_map *map)
 	 * complete.
 	 */
 
-	bpf_clear_redirect_map(map);
 	synchronize_rcu();
 
 	/* For cpu_map the remote CPUs can still be using the entries
@@ -602,7 +601,8 @@  static int cpu_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
 
 static int cpu_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
 {
-	return __bpf_xdp_redirect_map(map, ifindex, flags, __cpu_map_lookup_elem);
+	return __bpf_xdp_redirect_map(map, ifindex, flags, __cpu_map_lookup_elem,
+				      XDP_REDIR_CPU_MAP);
 }
 
 static int cpu_map_btf_id;
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index f7f42448259f..99f5670f7273 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -197,7 +197,6 @@  static void dev_map_free(struct bpf_map *map)
 	list_del_rcu(&dtab->list);
 	spin_unlock(&dev_map_lock);
 
-	bpf_clear_redirect_map(map);
 	synchronize_rcu();
 
 	/* Make sure prior __dev_map_entry_free() have completed. */
@@ -737,12 +736,14 @@  static int dev_map_hash_update_elem(struct bpf_map *map, void *key, void *value,
 
 static int dev_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
 {
-	return __bpf_xdp_redirect_map(map, ifindex, flags, __dev_map_lookup_elem);
+	return __bpf_xdp_redirect_map(map, ifindex, flags, __dev_map_lookup_elem,
+				      XDP_REDIR_DEV_MAP);
 }
 
 static int dev_hash_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
 {
-	return __bpf_xdp_redirect_map(map, ifindex, flags, __dev_map_hash_lookup_elem);
+	return __bpf_xdp_redirect_map(map, ifindex, flags, __dev_map_hash_lookup_elem,
+				      XDP_REDIR_DEV_MAP);
 }
 
 static int dev_map_btf_id;
diff --git a/net/core/filter.c b/net/core/filter.c
index 960299a3744f..cb6a6df3318b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3909,23 +3909,6 @@  static const struct bpf_func_proto bpf_xdp_adjust_meta_proto = {
 	.arg2_type	= ARG_ANYTHING,
 };
 
-static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
-			    struct bpf_map *map, struct xdp_buff *xdp)
-{
-	switch (map->map_type) {
-	case BPF_MAP_TYPE_DEVMAP:
-	case BPF_MAP_TYPE_DEVMAP_HASH:
-		return dev_map_enqueue(fwd, xdp, dev_rx);
-	case BPF_MAP_TYPE_CPUMAP:
-		return cpu_map_enqueue(fwd, xdp, dev_rx);
-	case BPF_MAP_TYPE_XSKMAP:
-		return __xsk_map_redirect(fwd, xdp);
-	default:
-		return -EBADRQC;
-	}
-	return 0;
-}
-
 void xdp_do_flush(void)
 {
 	__dev_flush();
@@ -3934,55 +3917,45 @@  void xdp_do_flush(void)
 }
 EXPORT_SYMBOL_GPL(xdp_do_flush);
 
-void bpf_clear_redirect_map(struct bpf_map *map)
-{
-	struct bpf_redirect_info *ri;
-	int cpu;
-
-	for_each_possible_cpu(cpu) {
-		ri = per_cpu_ptr(&bpf_redirect_info, cpu);
-		/* Avoid polluting remote cacheline due to writes if
-		 * not needed. Once we pass this test, we need the
-		 * cmpxchg() to make sure it hasn't been changed in
-		 * the meantime by remote CPU.
-		 */
-		if (unlikely(READ_ONCE(ri->map) == map))
-			cmpxchg(&ri->map, map, NULL);
-	}
-}
-
 int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
 		    struct bpf_prog *xdp_prog)
 {
 	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
-	struct bpf_map *map = READ_ONCE(ri->map);
-	u32 index = ri->tgt_index;
+	enum xdp_redirect_type type = ri->tgt_type;
 	void *fwd = ri->tgt_value;
 	int err;
 
-	ri->tgt_index = 0;
-	ri->tgt_value = NULL;
-	WRITE_ONCE(ri->map, NULL);
+	ri->tgt_type = XDP_REDIR_UNSET;
 
-	if (unlikely(!map)) {
-		fwd = dev_get_by_index_rcu(dev_net(dev), index);
+	switch (type) {
+	case XDP_REDIR_DEV_IFINDEX:
+		fwd = dev_get_by_index_rcu(dev_net(dev), (u32)(long)fwd);
 		if (unlikely(!fwd)) {
 			err = -EINVAL;
-			goto err;
+			break;
 		}
-
 		err = dev_xdp_enqueue(fwd, xdp, dev);
-	} else {
-		err = __bpf_tx_xdp_map(dev, fwd, map, xdp);
+		break;
+	case XDP_REDIR_DEV_MAP:
+		err = dev_map_enqueue(fwd, xdp, dev);
+		break;
+	case XDP_REDIR_CPU_MAP:
+		err = cpu_map_enqueue(fwd, xdp, dev);
+		break;
+	case XDP_REDIR_XSK_MAP:
+		err = __xsk_map_redirect(fwd, xdp);
+		break;
+	default:
+		err = -EBADRQC;
 	}
 
 	if (unlikely(err))
 		goto err;
 
-	_trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index);
+	_trace_xdp_redirect_map(dev, xdp_prog, fwd, type, ri);
 	return 0;
 err:
-	_trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err);
+	_trace_xdp_redirect_map_err(dev, xdp_prog, fwd, type, ri, err);
 	return err;
 }
 EXPORT_SYMBOL_GPL(xdp_do_redirect);
@@ -3991,41 +3964,37 @@  static int xdp_do_generic_redirect_map(struct net_device *dev,
 				       struct sk_buff *skb,
 				       struct xdp_buff *xdp,
 				       struct bpf_prog *xdp_prog,
-				       struct bpf_map *map)
+				       void *fwd,
+				       enum xdp_redirect_type type)
 {
 	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
-	u32 index = ri->tgt_index;
-	void *fwd = ri->tgt_value;
-	int err = 0;
-
-	ri->tgt_index = 0;
-	ri->tgt_value = NULL;
-	WRITE_ONCE(ri->map, NULL);
-
-	if (map->map_type == BPF_MAP_TYPE_DEVMAP ||
-	    map->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
-		struct bpf_dtab_netdev *dst = fwd;
+	int err;
 
-		err = dev_map_generic_redirect(dst, skb, xdp_prog);
+	switch (type) {
+	case XDP_REDIR_DEV_MAP:
+		err = dev_map_generic_redirect(fwd, skb, xdp_prog);
 		if (unlikely(err))
 			goto err;
-	} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
+		break;
+	case XDP_REDIR_XSK_MAP: {
 		struct xdp_sock *xs = fwd;
 
 		err = xsk_generic_rcv(xs, xdp);
 		if (err)
 			goto err;
 		consume_skb(skb);
-	} else {
+		break;
+	}
+	default:
 		/* TODO: Handle BPF_MAP_TYPE_CPUMAP */
 		err = -EBADRQC;
 		goto err;
 	}
 
-	_trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index);
+	_trace_xdp_redirect_map(dev, xdp_prog, fwd, type, ri);
 	return 0;
 err:
-	_trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err);
+	_trace_xdp_redirect_map_err(dev, xdp_prog, fwd, type, ri, err);
 	return err;
 }
 
@@ -4033,29 +4002,31 @@  int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 			    struct xdp_buff *xdp, struct bpf_prog *xdp_prog)
 {
 	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
-	struct bpf_map *map = READ_ONCE(ri->map);
-	u32 index = ri->tgt_index;
-	struct net_device *fwd;
+	enum xdp_redirect_type type = ri->tgt_type;
+	void *fwd = ri->tgt_value;
 	int err = 0;
 
-	if (map)
-		return xdp_do_generic_redirect_map(dev, skb, xdp, xdp_prog,
-						   map);
-	ri->tgt_index = 0;
-	fwd = dev_get_by_index_rcu(dev_net(dev), index);
-	if (unlikely(!fwd)) {
-		err = -EINVAL;
-		goto err;
-	}
+	ri->tgt_type = XDP_REDIR_UNSET;
+	ri->tgt_value = NULL;
 
-	err = xdp_ok_fwd_dev(fwd, skb->len);
-	if (unlikely(err))
-		goto err;
+	if (type == XDP_REDIR_DEV_IFINDEX) {
+		fwd = dev_get_by_index_rcu(dev_net(dev), (u32)(long)fwd);
+		if (unlikely(!fwd)) {
+			err = -EINVAL;
+			goto err;
+		}
 
-	skb->dev = fwd;
-	_trace_xdp_redirect(dev, xdp_prog, index);
-	generic_xdp_tx(skb, xdp_prog);
-	return 0;
+		err = xdp_ok_fwd_dev(fwd, skb->len);
+		if (unlikely(err))
+			goto err;
+
+		skb->dev = fwd;
+		_trace_xdp_redirect(dev, xdp_prog, index);
+		generic_xdp_tx(skb, xdp_prog);
+		return 0;
+	}
+
+	return xdp_do_generic_redirect_map(dev, skb, xdp, xdp_prog, fwd, type);
 err:
 	_trace_xdp_redirect_err(dev, xdp_prog, index, err);
 	return err;
@@ -4068,10 +4039,9 @@  BPF_CALL_2(bpf_xdp_redirect, u32, ifindex, u64, flags)
 	if (unlikely(flags))
 		return XDP_ABORTED;
 
-	ri->flags = flags;
-	ri->tgt_index = ifindex;
-	ri->tgt_value = NULL;
-	WRITE_ONCE(ri->map, NULL);
+	ri->tgt_type = XDP_REDIR_DEV_IFINDEX;
+	ri->tgt_index = 0;
+	ri->tgt_value = (void *)(long)ifindex;
 
 	return XDP_REDIRECT;
 }
diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c
index 711acb3636b3..2c58d88aa69d 100644
--- a/net/xdp/xskmap.c
+++ b/net/xdp/xskmap.c
@@ -87,7 +87,6 @@  static void xsk_map_free(struct bpf_map *map)
 {
 	struct xsk_map *m = container_of(map, struct xsk_map, map);
 
-	bpf_clear_redirect_map(map);
 	synchronize_net();
 	bpf_map_area_free(m);
 }
@@ -229,7 +228,8 @@  static int xsk_map_delete_elem(struct bpf_map *map, void *key)
 
 static int xsk_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
 {
-	return __bpf_xdp_redirect_map(map, ifindex, flags, __xsk_map_lookup_elem);
+	return __bpf_xdp_redirect_map(map, ifindex, flags, __xsk_map_lookup_elem,
+				      XDP_REDIR_XSK_MAP);
 }
 
 void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,