Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add XDPLua #7

Open
wants to merge 18 commits into
base: master-5.6
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[submodule "lib/lunatik"]
path = lib/lunatik
url = https://github.com/luainkernel/lunatik.git
[submodule "lib/luadata"]
path = lib/luadata
url = https://github.com/luainkernel/luadata
[submodule "lib/luaxdp"]
path = lib/luaxdp
url = https://github.com/luainkernel/luaxdp.git
[submodule "lib/luarcu"]
path = lib/luarcu
url = https://github.com/luainkernel/luarcu
[submodule "lib/luaunpack"]
path = lib/luaunpack
url = https://github.com/VictorNogueiraRio/luaunpack.git
4 changes: 4 additions & 0 deletions include/linux/netdevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -3772,6 +3772,10 @@ u32 __dev_xdp_query(struct net_device *dev, bpf_op_t xdp_op,
enum bpf_netdev_command cmd);
int xdp_umem_query(struct net_device *dev, u16 queue_id);

#ifdef CONFIG_XDP_LUA
int generic_xdp_lua_install_prog(char *lua_prog);
#endif /* CONFIG_XDP_LUA */

int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
int dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
bool is_skb_forwardable(const struct net_device *dev,
Expand Down
17 changes: 17 additions & 0 deletions include/net/xdp.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,30 @@ struct xdp_rxq_info {
struct xdp_mem_info mem;
} ____cacheline_aligned; /* perf critical, avoid false-sharing */

#ifdef CONFIG_XDP_LUA
struct xdplua_create_work {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add a field of luadata's datarefs into this struct to automate the reference control of these objects

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VictorNogueiraRio it makes sense to me.. also, we should rename this struct.. perhaps, xdplua_work.. create_work sounds like a function..

char lua_script[8192];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/8192/MAXSCRIPTLEN/

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XDPLUA_MAX_SCRIPT_LEN or XDPLUA_MAXSCRIPTLEN? any variation looks good for me.. 👍

struct lua_State *L;
struct work_struct work;
spinlock_t lock;
bool init;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/init/inuse/

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to add this comment ;-)

};

DECLARE_PER_CPU(struct xdplua_create_work, luaworks);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xdplua_works?

#endif /* CONFIG_XDP_LUA */


Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/\n///

struct xdp_buff {
void *data;
void *data_end;
void *data_meta;
void *data_hard_start;
unsigned long handle;
struct xdp_rxq_info *rxq;
#ifdef CONFIG_XDP_LUA
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the creation of struct xdplua_create_work there probably is no reason to modify struct xdp_buff

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed..

struct sk_buff *skb;
struct lua_State *L;
#endif /* CONFIG_XDP_LUA */
};

struct xdp_frame {
Expand Down
119 changes: 118 additions & 1 deletion include/uapi/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -2890,6 +2890,106 @@ union bpf_attr {
* Obtain the 64bit jiffies
* Return
* The 64 bit jiffies
*
* int bpf_lua_dataref(void *ctx, int offset)
* Description
* Create a new lua data buffer object pointing to the captured
* packet at the specified offset. The function leaves the new
* object on top of the Lua stack.
* Return
* Data object reference number on success, or -1 in case
* of failure
*
* void bpf_lua_dataunref(void *ctx, int data_ref)
* Description
* Releases the data-object reference, allowing it to be
* garbage-collected
*
* void bpf_lua_pcall(void *ctx, char *funcname, int num_args, int num_rets)
* Description
* Calls Lua function funcname with the given nargs arguments in protected mode
*
* void bpf_lua_pop(void *ctx, int n)
* Description
* Pops n elements from the Lua stack
*
* void bpf_lua_pushinteger(void *ctx, int num)
* Description
* Pushes an integer with value n onto the Lua stack.
*
* void bpf_lua_pushlightuserdata(void *ctx, void *ptr)
* Description
* Pushes a light userdata onto the Lua stack.
* Userdata represent C values in Lua.
* A light userdata represents a pointer, a void*.
* It is a value (like a number): you do not create it,
* it has no individual metatable, and it is not collected
* (as it was never created).
* A light userdata is equal to "any" light userdata with
* the same C address.
*
* void bpf_lua_pushlstring(void *ctx, const char *s, size_t len)
* Description
* Pushes the string pointed to by s with size len onto the stack.
* Lua makes (or reuses) an internal copy of the given string,
* so the memory at s can be freed or reused immediately after the
* function returns.
* The string can contain any binary data, including embedded zeros.
*
* void bpf_lua_pushmap(void *ctx, void *map)
* Description
* Pushes a BPF map onto the Lua stack
*
* void bpf_lua_pushskb(void *ctx)
* Description
* Pushes an SKB structure onto the Lua stack
*
* void bpf_lua_pushstring(void *ctx, const char *s)
* Description
* Pushes the zero-terminated string pointed to by s onto the stack.
* Lua makes (or reuses) an internal copy of the given string,
* so the memory at s can be freed or reused immediately after the
* function returns.
*
* int bpf_lua_toboolean(void *ctx, int index)
* Description
* Converts the Lua value at the given index to a C
* boolean value (0 or 1)
* Return
* 1 if the value in the given index of the Lua stack is
* different from from false or null, otherwise returns 0
*
* int bpf_lua_tointeger(void *ctx, int index)
* Description
* Converts the Lua value at the given index of the Lua stack
* to the signed integral type lua_Integer.
* Return
* The converted Lua value at the given index, if the value is
* convertible to an integer(see the Lua manual for more details
* on type conversion); otherwise returns 0
*
* void bpf_lua_tostring(void *ctx, const char *str, u32 size, int index)
* Description
* Converts the Lua value at the given index of the Lua
* stack to a C string and copies size bytes of it to
* value pointed by str
* Return
* 1 if the value at the given index of the Lua stack is a
* string; otherwise it returns 0
*
* int bpf_lua_type(void *ctx, int index)
* Description
* Obtains the type of the Lua value at the given index
* of the Lua stack
*
* Return
* Type of the value in the given valid index,
* or LUA_TNONE for a non-valid (but acceptable) index.
* The types returned by lua_type are coded by the
* following constants defined in lua.h: LUA_TNIL (0),
* LUA_TNUMBER, LUA_TBOOLEAN, LUA_TSTRING, LUA_TTABLE,
* LUA_TFUNCTION, LUA_TUSERDATA, LUA_TTHREAD, and
* LUA_TLIGHTUSERDATA.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
Expand Down Expand Up @@ -3010,7 +3110,24 @@ union bpf_attr {
FN(probe_read_kernel_str), \
FN(tcp_send_ack), \
FN(send_signal_thread), \
FN(jiffies64),
FN(jiffies64), \
/* #ifdef CONFIG_XDP_LUA */ \
FN(lua_dataref), \
FN(lua_dataunref), \
FN(lua_pcall), \
FN(lua_pop), \
FN(lua_pushinteger), \
FN(lua_pushlightuserdata), \
FN(lua_pushlstring), \
FN(lua_pushmap), \
FN(lua_pushskb), \
FN(lua_pushstring), \
FN(lua_toboolean), \
FN(lua_tointeger), \
FN(lua_newpacket), \
FN(lua_tostring), \
FN(lua_type),
/* #endif CONFIG_XDP_LUA */

/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
Expand Down
3 changes: 3 additions & 0 deletions include/uapi/linux/if_link.h
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,9 @@ enum {
IFLA_XDP_DRV_PROG_ID,
IFLA_XDP_SKB_PROG_ID,
IFLA_XDP_HW_PROG_ID,
#ifdef CONFIG_XDP_LUA
IFLA_XDP_LUA_PROG,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should uniformize our "namespace".. it looks like we should use XDP_LUA everywhere instead of XDPLUA, right?

#endif /* CONFIG_XDP_LUA */
__IFLA_XDP_MAX,
};

Expand Down
6 changes: 6 additions & 0 deletions init/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,12 @@ config HAVE_PCSPKR_PLATFORM
config BPF
bool

config LUNATIK
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we put this config on Lunatik module itself?

bool "Lunatik"
default n
help
Support for the Lua interpreter

menuconfig EXPERT
bool "Configure standard kernel features (expert users)"
# Unhide debug options, to make the on-by-default options visible
Expand Down
8 changes: 8 additions & 0 deletions lib/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,11 @@ obj-$(CONFIG_OBJAGG) += objagg.o

# KUnit tests
obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
subdir-ccflags-y += -I$(srctree)/lib/lunatik/lua \
-I$(srctree)/lib/luadata/ -I$(srctree)/lib/luaunpack/ \
-D_KERNEL
obj-$(CONFIG_LUNATIK) += lunatik/
obj-$(CONFIG_LUADATA) += luadata/
obj-$(CONFIG_LUAXDP) += luaxdp/
obj-$(CONFIG_LUARCU) += luarcu/
obj-$(CONFIG_LUAUNPACK) += luaunpack/
1 change: 1 addition & 0 deletions lib/luadata
Submodule luadata added at 21137a
1 change: 1 addition & 0 deletions lib/luarcu
Submodule luarcu added at bc563d
1 change: 1 addition & 0 deletions lib/luaunpack
Submodule luaunpack added at 0589fc
1 change: 1 addition & 0 deletions lib/luaxdp
Submodule luaxdp added at 64216b
1 change: 1 addition & 0 deletions lib/lunatik
Submodule lunatik added at 0574a8
4 changes: 4 additions & 0 deletions net/core/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ obj-y := sock.o request_sock.o skbuff.o datagram.o stream.o scm.o \

obj-$(CONFIG_SYSCTL) += sysctl_net_core.o

CFLAGS_dev.o = -Ilib/lunatik/lua/ -D_KERNEL \
-Ilib/luadata/
CFLAGS_filter.o = -Ilib/lunatik/lua/ -D_KERNEL \
-Ilib/luadata/ -Ilib/luaunpack/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: should luaunpack go on master?

obj-y += dev.o dev_addr_lists.o dst.o netevent.o \
neighbour.o rtnetlink.o utils.o link_watch.o filter.o \
sock_diag.o dev_ioctl.o tso.o sock_reuseport.o \
Expand Down
72 changes: 72 additions & 0 deletions net/core/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@
* - netif_rx() feedback
*/

#ifdef CONFIG_XDP_LUA
#include <lua.h>
#include <lauxlib.h>
#include <lstate.h>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need lstate.h here? it looks weird to me..

#include <lualib.h>
#include <luadata.h>
#endif /* CONFIG_XDP_LUA */

#include <linux/uaccess.h>
#include <linux/bitops.h>
#include <linux/capability.h>
Expand Down Expand Up @@ -152,6 +160,9 @@

static DEFINE_SPINLOCK(ptype_lock);
static DEFINE_SPINLOCK(offload_lock);
#ifdef CONFIG_XDP_LUA
DEFINE_PER_CPU(struct xdplua_create_work, luaworks);
#endif
struct list_head ptype_base[PTYPE_HASH_SIZE] __read_mostly;
struct list_head ptype_all __read_mostly; /* Taps */
static struct list_head offload_base __read_mostly;
Expand Down Expand Up @@ -4512,6 +4523,9 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
bool orig_bcast;
int hlen, off;
u32 mac_len;
#ifdef CONFIG_XDP_LUA
struct xdplua_create_work *lw;
#endif /* CONFIG_XDP_LUA */

/* Reinjected packets coming from act_mirred or similar should
* not get XDP generic processing.
Expand Down Expand Up @@ -4556,9 +4570,22 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,

rxqueue = netif_get_rxqueue(skb);
xdp->rxq = &rxqueue->xdp_rxq;
#ifdef CONFIG_XDP_LUA
lw = this_cpu_ptr(&luaworks);

xdp->skb = skb;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will get rid of this, right? so, I would remove the \n above as well..

xdp->L = lw->L;
#endif /* CONFIG_XDP_LUA */

act = bpf_prog_run_xdp(xdp_prog, xdp);

#ifdef CONFIG_XDP_LUA
if (lw->init) {
lw->init = false;
spin_unlock(&lw->lock);
}
#endif /* CONFIG_XDP_LUA */

/* check if bpf_xdp_adjust_head was used */
off = xdp->data - orig_data;
if (off) {
Expand Down Expand Up @@ -5366,6 +5393,35 @@ static int generic_xdp_install(struct net_device *dev, struct netdev_bpf *xdp)
return ret;
}

#ifdef CONFIG_XDP_LUA

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/\n//

static void per_cpu_xdp_lua_install(struct work_struct *w) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/per_cpu_xdp_lua_install/xdp_lua_install_per_cpu/

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, s/ {/\r{\r/

int this_cpu = smp_processor_id();
struct xdplua_create_work *lw =
container_of(w, struct xdplua_create_work, work);

spin_lock_bh(&lw->lock);
if (luaL_dostring(lw->L, lw->lua_script)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/lua_script/script/

pr_err(KERN_INFO "error: %s\nOn cpu: %d\n",
lua_tostring(lw->L, -1), this_cpu);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to pop the string out of the stack, right?

}
spin_unlock_bh(&lw->lock);
}

int generic_xdp_lua_install_prog(char *lua_script)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/lua_script/script/

{
int cpu;
struct xdplua_create_work *lw;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this variable can be moved to the for_each scope, right?


for_each_possible_cpu(cpu) {
lw = per_cpu_ptr(&luaworks, cpu);
strcpy(lw->lua_script, lua_script);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strncpy(lw->lua_script, lua_script, MAXSCRIPTLEN);

schedule_work_on(cpu, &lw->work);
}
return 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you can't fail, why not using void? is it some contract?

}
#endif /* CONFIG_XDP_LUA */

static int netif_receive_skb_internal(struct sk_buff *skb)
{
int ret;
Expand Down Expand Up @@ -10487,6 +10543,9 @@ static int __init net_dev_init(void)
for_each_possible_cpu(i) {
struct work_struct *flush = per_cpu_ptr(&flush_works, i);
struct softnet_data *sd = &per_cpu(softnet_data, i);
#ifdef CONFIG_XDP_LUA
struct xdplua_create_work *lw = per_cpu_ptr(&luaworks, i);
#endif

INIT_WORK(flush, flush_backlog);

Expand All @@ -10506,6 +10565,19 @@ static int __init net_dev_init(void)
init_gro_hash(&sd->backlog);
sd->backlog.poll = process_backlog;
sd->backlog.weight = weight_p;
#ifdef CONFIG_XDP_LUA
lw->L = luaL_newstate();
WARN_ON(!lw->L);

if (!lw->L)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to improve this, @lneto any suggestions?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, we should fail XDPLua initiation completely and destroy all states we have created so far.. and, of course, disable XDPLua execution in runtime putting a guard..

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, another example of what I was arguing on slack is to move all this code to a xdplua_init() function

continue;

luaL_openlibs(lw->L);
luaL_requiref(lw->L, "data", luaopen_data, 1);
lua_pop(lw->L, 1);

INIT_WORK(&lw->work, per_cpu_xdp_lua_install);
#endif /* CONFIG_XDP_LUA */
}

dev_boot_phase = 0;
Expand Down
Loading