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

Fix issues with pinned sk_storage map #3833

Merged
merged 1 commit into from
Feb 2, 2022
Merged

Fix issues with pinned sk_storage map #3833

merged 1 commit into from
Feb 2, 2022

Conversation

yonghong-song
Copy link
Collaborator

@yonghong-song yonghong-song commented Feb 2, 2022

The BPF_SK_STORAGE map is defined as below:

  #define BPF_SK_STORAGE(_name, _leaf_type) \
  struct _name##_table_t { \
    int key; \
    _leaf_type leaf; \
    void * (*sk_storage_get) (void *, void *, int); \
    int (*sk_storage_delete) (void *); \
    u32 flags; \
  };

The structure has two func pointer members, sk_storage_get and
sk_storage_delete, which will be translated to corresponding
bpf helpers by rewriter.

If a BPF_SK_STORAGE map is pinned to bpffs, the map can be
used in another bcc based process with BPF_TABLE_PINNED macro.

  #define BPF_TABLE_PINNED(_table_type, _key_type, _leaf_type, _name, _max_entries, _pinned) \
  BPF_TABLE(_table_type ":" _pinned, _key_type, _leaf_type, _name, _max_entries)

Miao Xu reported an issue such that the structure supporting BPF_TABLE
does not have func pointer member sk_storage_get and sk_storage_delete and
this may cause BPF program compilation failure. For example,
without helpers.h change in this patch, the test case in this patch will have the
following error:

  /virtual/main.c:14:22: error: no member named 'sk_storage_get' in 'struct sk_stg_table_t'
          val = sk_stg.sk_storage_get(sk, NULL, BPF_SK_STORAGE_GET_F_CREATE);

To fix the above issue, we need to add sk_storage_get and
sk_storage_delete to BPF_F_TABLE macro. I also added
{task, inode}storage{get, delete} func pointer members
to BPF_F_TABLE macro. Some other map macros like BPF_SOCKMAP,
BPF_SOCKHASH, etc. also have special func pointer members.
But I leave them for future patches if we have use cases for it.

Signed-off-by: Yonghong Song [email protected]

The BPF_SK_STORAGE map is defined as below:
  #define BPF_SK_STORAGE(_name, _leaf_type) \
  struct _name##_table_t { \
    int key; \
    _leaf_type leaf; \
    void * (*sk_storage_get) (void *, void *, int); \
    int (*sk_storage_delete) (void *); \
    u32 flags; \
  };
The structure has two func pointer members, sk_storage_get and
sk_storage_delete, which will be translated to corresponding
bpf helpers by rewriter.

If a BPF_SK_STORAGE map is pinned to bpffs, the map can be
used in another bcc based process with BPF_TABLE_PINNED macro.
  #define BPF_TABLE_PINNED(_table_type, _key_type, _leaf_type, _name, _max_entries, _pinned) \
  BPF_TABLE(_table_type ":" _pinned, _key_type, _leaf_type, _name, _max_entries)

Miao Xu reported an issue such that the structure supporting BPF_TABLE
does not have func pointer member sk_storage_get and sk_storage_delete and
this may cause BPF program compilation failure. For example,
without helpers.h change in this patch, the test case in this patch will have the
following error:
  /virtual/main.c:14:22: error: no member named 'sk_storage_get' in 'struct sk_stg_table_t'
          val = sk_stg.sk_storage_get(sk, NULL, BPF_SK_STORAGE_GET_F_CREATE);

To fix the above issue, we need to add sk_storage_get and
sk_storage_delete to BPF_F_TABLE macro. I also added
{task, inode}_storage_{get, delete} func pointer members
to BPF_F_TABLE macro. Some other map macros like BPF_SOCKMAP,
BPF_SOCKHASH, etc. also have special func pointer members.
But I leave them for future patches if we have use cases for it.

Signed-off-by: Yonghong Song <[email protected]>
@davemarchevsky
Copy link
Collaborator

Thanks for the detailed explanation! LGTM

To confirm my understanding, is this the rewriter magic you're referring to?

@davemarchevsky davemarchevsky merged commit 63de4c5 into master Feb 2, 2022
@yonghong-song
Copy link
Collaborator Author

To confirm my understanding, is this the rewriter magic you're referring to?

Yes, it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants