* [PATCH v9] mm: support madvise(MADV_FREE)
From: Minchan Kim @ 2014-07-01 0:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-mm, Minchan Kim, Michael Kerrisk, Linux API,
Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
Mel Gorman, Jason Evans, Zhang Yanfei
Linux doesn't have an ability to free pages lazy while other OS
already have been supported that named by madvise(MADV_FREE).
The gain is clear that kernel can discard freed pages rather than
swapping out or OOM if memory pressure happens.
Without memory pressure, freed pages would be reused by userspace
without another additional overhead(ex, page fault + allocation
+ zeroing).
How to work is following as.
When madvise syscall is called, VM clears dirty bit of ptes of
the range. If memory pressure happens, VM checks dirty bit of
page table and if it found still "clean", it means it's a
"lazyfree pages" so VM could discard the page instead of swapping out.
Once there was store operation for the page before VM peek a page
to reclaim, dirty bit is set so VM can swap out the page instead of
discarding.
Firstly, heavy users would be general allocators(ex, jemalloc,
tcmalloc and hope glibc supports it) and jemalloc/tcmalloc already
have supported the feature for other OS(ex, FreeBSD)
barrios@blaptop:~/benchmark/ebizzy$ lscpu
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Byte Order: Little Endian
CPU(s): 4
On-line CPU(s) list: 0-3
Thread(s) per core: 2
Core(s) per socket: 2
Socket(s): 1
NUMA node(s): 1
Vendor ID: GenuineIntel
CPU family: 6
Model: 42
Stepping: 7
CPU MHz: 2801.000
BogoMIPS: 5581.64
Virtualization: VT-x
L1d cache: 32K
L1i cache: 32K
L2 cache: 256K
L3 cache: 4096K
NUMA node0 CPU(s): 0-3
ebizzy benchmark(./ebizzy -S 10 -n 512)
vanilla-jemalloc MADV_free-jemalloc
1 thread
records: 10 records: 10
avg: 7682.10 avg: 15306.10
std: 62.35(0.81%) std: 347.99(2.27%)
max: 7770.00 max: 15622.00
min: 7598.00 min: 14772.00
2 thread
records: 10 records: 10
avg: 12747.50 avg: 24171.00
std: 792.06(6.21%) std: 895.18(3.70%)
max: 13337.00 max: 26023.00
min: 10535.00 min: 23152.00
4 thread
records: 10 records: 10
avg: 16474.60 avg: 33717.90
std: 1496.45(9.08%) std: 2008.97(5.96%)
max: 17877.00 max: 35958.00
min: 12224.00 min: 29565.00
8 thread
records: 10 records: 10
avg: 16778.50 avg: 33308.10
std: 825.53(4.92%) std: 1668.30(5.01%)
max: 17543.00 max: 36010.00
min: 14576.00 min: 29577.00
16 thread
records: 10 records: 10
avg: 20614.40 avg: 35516.30
std: 602.95(2.92%) std: 1283.65(3.61%)
max: 21753.00 max: 37178.00
min: 19605.00 min: 33217.00
32 thread
records: 10 records: 10
avg: 22771.70 avg: 36018.50
std: 598.94(2.63%) std: 1046.76(2.91%)
max: 24035.00 max: 37266.00
min: 22108.00 min: 34149.00
In summary, MADV_FREE is about 2 time faster than MADV_DONTNEED.
* From v8
* Rebased-on v3.16-rc2-mmotm-2014-06-25-16-44
* From v7
* Rebased-on next-20140613
* From v6
* Remove page from swapcache in syscal time
* Move utility functions from memory.c to madvise.c - Johannes
* Rename untilify functtions - Johannes
* Remove unnecessary checks from vmscan.c - Johannes
* Rebased-on v3.15-rc5-mmotm-2014-05-16-16-56
* Drop Reviewe-by because there was some changes since then.
* From v5
* Fix PPC problem which don't flush TLB - Rik
* Remove unnecessary lazyfree_range stub function - Rik
* Rebased on v3.15-rc5
* From v4
* Add Reviewed-by: Zhang Yanfei
* Rebase on v3.15-rc1-mmotm-2014-04-15-16-14
* From v3
* Add "how to work part" in description - Zhang
* Add page_discardable utility function - Zhang
* Clean up
* From v2
* Remove forceful dirty marking of swap-readed page - Johannes
* Remove deactivation logic of lazyfreed page
* Rebased on 3.14
* Remove RFC tag
* From v1
* Use custom page table walker for madvise_free - Johannes
* Remove PG_lazypage flag - Johannes
* Do madvise_dontneed instead of madvise_freein swapless system
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Linux API <linux-api@vger.kernel.org>
Cc: Hugh Dickins <hughd@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Jason Evans <je@fb.com>
Cc: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
include/linux/rmap.h | 8 +-
include/linux/vm_event_item.h | 1 +
include/uapi/asm-generic/mman-common.h | 1 +
mm/madvise.c | 174 +++++++++++++++++++++++++++++++++
mm/rmap.c | 34 ++++++-
mm/vmscan.c | 37 +++++--
mm/vmstat.c | 1 +
7 files changed, 245 insertions(+), 11 deletions(-)
diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index be574506e6a9..dea05914f167 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -181,7 +181,8 @@ static inline void page_dup_rmap(struct page *page)
* Called from mm/vmscan.c to handle paging out
*/
int page_referenced(struct page *, int is_locked,
- struct mem_cgroup *memcg, unsigned long *vm_flags);
+ struct mem_cgroup *memcg, unsigned long *vm_flags,
+ int *is_dirty);
#define TTU_ACTION(x) ((x) & TTU_ACTION_MASK)
@@ -260,9 +261,12 @@ int rmap_walk(struct page *page, struct rmap_walk_control *rwc);
static inline int page_referenced(struct page *page, int is_locked,
struct mem_cgroup *memcg,
- unsigned long *vm_flags)
+ unsigned long *vm_flags,
+ int *is_pte_dirty)
{
*vm_flags = 0;
+ if (is_pte_dirty)
+ *is_pte_dirty = 0;
return 0;
}
diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index ced92345c963..e2d3fb1e9814 100644
--- a/include/linux/vm_event_item.h
+++ b/include/linux/vm_event_item.h
@@ -25,6 +25,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
FOR_ALL_ZONES(PGALLOC),
PGFREE, PGACTIVATE, PGDEACTIVATE,
PGFAULT, PGMAJFAULT,
+ PGLAZYFREED,
FOR_ALL_ZONES(PGREFILL),
FOR_ALL_ZONES(PGSTEAL_KSWAPD),
FOR_ALL_ZONES(PGSTEAL_DIRECT),
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index ddc3b36f1046..7a94102b7a02 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -34,6 +34,7 @@
#define MADV_SEQUENTIAL 2 /* expect sequential page references */
#define MADV_WILLNEED 3 /* will need these pages */
#define MADV_DONTNEED 4 /* don't need these pages */
+#define MADV_FREE 5 /* free pages only if memory pressure */
/* common parameters: try to keep these consistent across architectures */
#define MADV_REMOVE 9 /* remove these pages & resources */
diff --git a/mm/madvise.c b/mm/madvise.c
index 0938b30da4ab..372a25a8ea82 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -19,6 +19,9 @@
#include <linux/blkdev.h>
#include <linux/swap.h>
#include <linux/swapops.h>
+#include <linux/mmu_notifier.h>
+
+#include <asm/tlb.h>
/*
* Any behaviour which results in changes to the vma->vm_flags needs to
@@ -31,6 +34,7 @@ static int madvise_need_mmap_write(int behavior)
case MADV_REMOVE:
case MADV_WILLNEED:
case MADV_DONTNEED:
+ case MADV_FREE:
return 0;
default:
/* be safe, default to 1. list exceptions explicitly */
@@ -251,6 +255,168 @@ static long madvise_willneed(struct vm_area_struct *vma,
return 0;
}
+static unsigned long madvise_free_pte_range(struct mmu_gather *tlb,
+ struct vm_area_struct *vma, pmd_t *pmd,
+ unsigned long addr, unsigned long end)
+{
+ struct mm_struct *mm = tlb->mm;
+ spinlock_t *ptl;
+ pte_t *start_pte;
+ pte_t *pte;
+ struct page *page;
+
+ start_pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
+ pte = start_pte;
+ arch_enter_lazy_mmu_mode();
+ do {
+ pte_t ptent = *pte;
+
+ if (pte_none(ptent))
+ continue;
+
+ if (!pte_present(ptent))
+ continue;
+
+ page = vm_normal_page(vma, addr, ptent);
+ if (page && PageSwapCache(page)) {
+ if (trylock_page(page)) {
+ if (try_to_free_swap(page))
+ ClearPageDirty(page);
+ unlock_page(page);
+ } else
+ continue;
+ }
+
+ /*
+ * Some of architecture(ex, PPC) don't update TLB
+ * with set_pte_at and tlb_remove_tlb_entry so for
+ * the portability, remap the pte with old|clean
+ * after pte clearing.
+ */
+ ptent = ptep_get_and_clear_full(mm, addr, pte,
+ tlb->fullmm);
+ ptent = pte_mkold(ptent);
+ ptent = pte_mkclean(ptent);
+ set_pte_at(mm, addr, pte, ptent);
+ tlb_remove_tlb_entry(tlb, pte, addr);
+ } while (pte++, addr += PAGE_SIZE, addr != end);
+ arch_leave_lazy_mmu_mode();
+ pte_unmap_unlock(start_pte, ptl);
+
+ return addr;
+}
+
+static inline unsigned long madvise_free_pmd_range(struct mmu_gather *tlb,
+ struct vm_area_struct *vma, pud_t *pud,
+ unsigned long addr, unsigned long end)
+{
+ pmd_t *pmd;
+ unsigned long next;
+
+ pmd = pmd_offset(pud, addr);
+ do {
+ /*
+ * XXX: We can optimize with supporting Hugepage free
+ * if the range covers.
+ */
+ next = pmd_addr_end(addr, end);
+ if (pmd_trans_huge(*pmd))
+ split_huge_page_pmd(vma, addr, pmd);
+ /*
+ * Here there can be other concurrent MADV_DONTNEED or
+ * trans huge page faults running, and if the pmd is
+ * none or trans huge it can change under us. This is
+ * because MADV_LAZYFREE holds the mmap_sem in read
+ * mode.
+ */
+ if (pmd_none_or_trans_huge_or_clear_bad(pmd))
+ goto next;
+ next = madvise_free_pte_range(tlb, vma, pmd, addr, next);
+next:
+ cond_resched();
+ } while (pmd++, addr = next, addr != end);
+
+ return addr;
+}
+
+static inline unsigned long madvise_free_pud_range(struct mmu_gather *tlb,
+ struct vm_area_struct *vma, pgd_t *pgd,
+ unsigned long addr, unsigned long end)
+{
+ pud_t *pud;
+ unsigned long next;
+
+ pud = pud_offset(pgd, addr);
+ do {
+ next = pud_addr_end(addr, end);
+ if (pud_none_or_clear_bad(pud))
+ continue;
+ next = madvise_free_pmd_range(tlb, vma, pud, addr, next);
+ } while (pud++, addr = next, addr != end);
+
+ return addr;
+}
+
+static void madvise_free_page_range(struct mmu_gather *tlb,
+ struct vm_area_struct *vma,
+ unsigned long addr, unsigned long end)
+{
+ pgd_t *pgd;
+ unsigned long next;
+
+ BUG_ON(addr >= end);
+ tlb_start_vma(tlb, vma);
+ pgd = pgd_offset(vma->vm_mm, addr);
+ do {
+ next = pgd_addr_end(addr, end);
+ if (pgd_none_or_clear_bad(pgd))
+ continue;
+ next = madvise_free_pud_range(tlb, vma, pgd, addr, next);
+ } while (pgd++, addr = next, addr != end);
+ tlb_end_vma(tlb, vma);
+}
+
+static int madvise_free_single_vma(struct vm_area_struct *vma,
+ unsigned long start_addr, unsigned long end_addr)
+{
+ unsigned long start, end;
+ struct mm_struct *mm = vma->vm_mm;
+ struct mmu_gather tlb;
+
+ if (vma->vm_flags & (VM_LOCKED|VM_HUGETLB|VM_PFNMAP))
+ return -EINVAL;
+
+ /* MADV_FREE works for only anon vma at the moment */
+ if (vma->vm_file)
+ return -EINVAL;
+
+ start = max(vma->vm_start, start_addr);
+ if (start >= vma->vm_end)
+ return -EINVAL;
+ end = min(vma->vm_end, end_addr);
+ if (end <= vma->vm_start)
+ return -EINVAL;
+
+ lru_add_drain();
+ tlb_gather_mmu(&tlb, mm, start, end);
+ update_hiwater_rss(mm);
+
+ mmu_notifier_invalidate_range_start(mm, start, end);
+ madvise_free_page_range(&tlb, vma, start, end);
+ mmu_notifier_invalidate_range_end(mm, start, end);
+ tlb_finish_mmu(&tlb, start, end);
+
+ return 0;
+}
+
+static long madvise_free(struct vm_area_struct *vma,
+ struct vm_area_struct **prev,
+ unsigned long start, unsigned long end)
+{
+ *prev = vma;
+ return madvise_free_single_vma(vma, start, end);
+}
+
/*
* Application no longer needs these pages. If the pages are dirty,
* it's OK to just throw them away. The app will be more careful about
@@ -381,6 +547,13 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
return madvise_remove(vma, prev, start, end);
case MADV_WILLNEED:
return madvise_willneed(vma, prev, start, end);
+ case MADV_FREE:
+ /*
+ * XXX: In this implementation, MADV_FREE works like
+ * MADV_DONTNEED on swapless system or full swap.
+ */
+ if (get_nr_swap_pages() > 0)
+ return madvise_free(vma, prev, start, end);
case MADV_DONTNEED:
return madvise_dontneed(vma, prev, start, end);
default:
@@ -400,6 +573,7 @@ madvise_behavior_valid(int behavior)
case MADV_REMOVE:
case MADV_WILLNEED:
case MADV_DONTNEED:
+ case MADV_FREE:
#ifdef CONFIG_KSM
case MADV_MERGEABLE:
case MADV_UNMERGEABLE:
diff --git a/mm/rmap.c b/mm/rmap.c
index 7928ddd91b6e..ee495d84c8b3 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -663,6 +663,7 @@ int page_mapped_in_vma(struct page *page, struct vm_area_struct *vma)
}
struct page_referenced_arg {
+ int dirtied;
int mapcount;
int referenced;
unsigned long vm_flags;
@@ -677,6 +678,7 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
struct mm_struct *mm = vma->vm_mm;
spinlock_t *ptl;
int referenced = 0;
+ int dirty = 0;
struct page_referenced_arg *pra = arg;
if (unlikely(PageTransHuge(page))) {
@@ -729,6 +731,10 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
if (likely(!(vma->vm_flags & VM_SEQ_READ)))
referenced++;
}
+
+ if (pte_dirty(*pte))
+ dirty++;
+
pte_unmap_unlock(pte, ptl);
}
@@ -737,6 +743,9 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
pra->vm_flags |= vma->vm_flags;
}
+ if (dirty)
+ pra->dirtied++;
+
pra->mapcount--;
if (!pra->mapcount)
return SWAP_SUCCESS; /* To break the loop */
@@ -761,6 +770,7 @@ static bool invalid_page_referenced_vma(struct vm_area_struct *vma, void *arg)
* @is_locked: caller holds lock on the page
* @memcg: target memory cgroup
* @vm_flags: collect encountered vma->vm_flags who actually referenced the page
+ * @is_pte_dirty: ptes which have marked dirty bit - used for lazyfree page
*
* Quick test_and_clear_referenced for all mappings to a page,
* returns the number of ptes which referenced the page.
@@ -768,7 +778,8 @@ static bool invalid_page_referenced_vma(struct vm_area_struct *vma, void *arg)
int page_referenced(struct page *page,
int is_locked,
struct mem_cgroup *memcg,
- unsigned long *vm_flags)
+ unsigned long *vm_flags,
+ int *is_pte_dirty)
{
int ret;
int we_locked = 0;
@@ -783,6 +794,9 @@ int page_referenced(struct page *page,
};
*vm_flags = 0;
+ if (is_pte_dirty)
+ *is_pte_dirty = 0;
+
if (!page_mapped(page))
return 0;
@@ -810,6 +824,9 @@ int page_referenced(struct page *page,
if (we_locked)
unlock_page(page);
+ if (is_pte_dirty)
+ *is_pte_dirty = pra.dirtied;
+
return pra.referenced;
}
@@ -1128,6 +1145,7 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
spinlock_t *ptl;
int ret = SWAP_AGAIN;
enum ttu_flags flags = (enum ttu_flags)arg;
+ int dirty = 0;
pte = page_check_address(page, mm, address, &ptl, 0);
if (!pte)
@@ -1157,7 +1175,8 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
pteval = ptep_clear_flush(vma, address, pte);
/* Move the dirty bit to the physical page now the pte is gone. */
- if (pte_dirty(pteval))
+ dirty = pte_dirty(pteval);
+ if (dirty)
set_page_dirty(page);
/* Update high watermark before we lower rss */
@@ -1204,6 +1223,16 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
}
dec_mm_counter(mm, MM_ANONPAGES);
inc_mm_counter(mm, MM_SWAPENTS);
+ } else if (flags & TTU_UNMAP) {
+ if (dirty || PageDirty(page)) {
+ set_pte_at(mm, address, pte, pteval);
+ ret = SWAP_FAIL;
+ goto out_unmap;
+ } else {
+ /* It's a freeable page by madvise_free */
+ dec_mm_counter(mm, MM_ANONPAGES);
+ goto discard;
+ }
} else if (IS_ENABLED(CONFIG_MIGRATION)) {
/*
* Store the pfn of the page in a special migration
@@ -1227,6 +1256,7 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
} else
dec_mm_counter(mm, MM_FILEPAGES);
+discard:
page_remove_rmap(page);
page_cache_release(page);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 6d24fd63b209..f7a45600846f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -707,13 +707,17 @@ enum page_references {
};
static enum page_references page_check_references(struct page *page,
- struct scan_control *sc)
+ struct scan_control *sc,
+ bool *freeable)
{
int referenced_ptes, referenced_page;
unsigned long vm_flags;
+ int pte_dirty;
+
+ VM_BUG_ON_PAGE(!PageLocked(page), page);
referenced_ptes = page_referenced(page, 1, sc->target_mem_cgroup,
- &vm_flags);
+ &vm_flags, &pte_dirty);
referenced_page = TestClearPageReferenced(page);
/*
@@ -754,6 +758,10 @@ static enum page_references page_check_references(struct page *page,
return PAGEREF_KEEP;
}
+ if (PageAnon(page) && !pte_dirty && !PageSwapCache(page) &&
+ !PageDirty(page))
+ *freeable = true;
+
/* Reclaim if clean, defer dirty pages to writeback */
if (referenced_page && !PageSwapBacked(page))
return PAGEREF_RECLAIM_CLEAN;
@@ -823,6 +831,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
int may_enter_fs;
enum page_references references = PAGEREF_RECLAIM_CLEAN;
bool dirty, writeback;
+ bool freeable = false;
cond_resched();
@@ -945,7 +954,8 @@ static unsigned long shrink_page_list(struct list_head *page_list,
}
if (!force_reclaim)
- references = page_check_references(page, sc);
+ references = page_check_references(page, sc,
+ &freeable);
switch (references) {
case PAGEREF_ACTIVATE:
@@ -961,7 +971,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
* Anonymous process memory has backing store?
* Try to allocate it some swap space here.
*/
- if (PageAnon(page) && !PageSwapCache(page)) {
+ if (PageAnon(page) && !PageSwapCache(page) && !freeable) {
if (!(sc->gfp_mask & __GFP_IO))
goto keep_locked;
if (!add_to_swap(page, page_list))
@@ -976,7 +986,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
* The page is mapped into the page tables of one or more
* processes. Try to unmap it here.
*/
- if (page_mapped(page) && mapping) {
+ if (page_mapped(page) && (mapping || freeable)) {
switch (try_to_unmap(page, ttu_flags)) {
case SWAP_FAIL:
goto activate_locked;
@@ -985,7 +995,20 @@ static unsigned long shrink_page_list(struct list_head *page_list,
case SWAP_MLOCK:
goto cull_mlocked;
case SWAP_SUCCESS:
- ; /* try to free the page below */
+ /* try to free the page below */
+ if (!freeable)
+ break;
+ /*
+ * Freeable anon page doesn't have mapping
+ * due to skipping of swapcache so we free
+ * page in here rather than __remove_mapping.
+ */
+ VM_BUG_ON_PAGE(PageSwapCache(page), page);
+ if (!page_freeze_refs(page, 1))
+ goto keep_locked;
+ __clear_page_locked(page);
+ count_vm_event(PGLAZYFREED);
+ goto free_it;
}
}
@@ -1727,7 +1750,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
}
if (page_referenced(page, 0, sc->target_mem_cgroup,
- &vm_flags)) {
+ &vm_flags, NULL)) {
nr_rotated += hpage_nr_pages(page);
/*
* Identify referenced, file-backed active pages and
diff --git a/mm/vmstat.c b/mm/vmstat.c
index eef6321c8470..da18337c6c66 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -794,6 +794,7 @@ const char * const vmstat_text[] = {
"pgfault",
"pgmajfault",
+ "pglazyfreed",
TEXTS_FOR_ZONES("pgrefill")
TEXTS_FOR_ZONES("pgsteal_kswapd")
--
2.0.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH v3 0/2] block: virtio-blk: support multi vq per virtio-blk
From: Ming Lei @ 2014-07-01 1:36 UTC (permalink / raw)
To: Jens Axboe, Linux Kernel Mailing List
Cc: Rusty Russell, linux-api-u79uwXL29TY76Z2rM5mHXA,
Linux Virtualization, Michael S. Tsirkin, Stefan Hajnoczi,
Paolo Bonzini
In-Reply-To: <CACVXFVMHPdEzai32jRmLt7qGMFcUqFb8OunF0tzySpKoLWWiwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Jens and Rusty,
On Thu, Jun 26, 2014 at 8:04 PM, Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> On Thu, Jun 26, 2014 at 5:41 PM, Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
>> Hi,
>>
>> These patches try to support multi virtual queues(multi-vq) in one
>> virtio-blk device, and maps each virtual queue(vq) to blk-mq's
>> hardware queue.
>>
>> With this approach, both scalability and performance on virtio-blk
>> device can get improved.
>>
>> For verifying the improvement, I implements virtio-blk multi-vq over
>> qemu's dataplane feature, and both handling host notification
>> from each vq and processing host I/O are still kept in the per-device
>> iothread context, the change is based on qemu v2.0.0 release, and
>> can be accessed from below tree:
>>
>> git://kernel.ubuntu.com/ming/qemu.git #v2.0.0-virtblk-mq.1
>>
>> For enabling the multi-vq feature, 'num_queues=N' need to be added into
>> '-device virtio-blk-pci ...' of qemu command line, and suggest to pass
>> 'vectors=N+1' to keep one MSI irq vector per each vq, and the feature
>> depends on x-data-plane.
>>
>> Fio(libaio, randread, iodepth=64, bs=4K, jobs=N) is run inside VM to
>> verify the improvement.
>>
>> I just create a small quadcore VM and run fio inside the VM, and
>> num_queues of the virtio-blk device is set as 2, but looks the
>> improvement is still obvious. The host is 2 sockets, 8cores(16threads)
>> server.
>>
>> 1), about scalability
>> - jobs = 2, thoughput: +33%
>> - jobs = 4, thoughput: +100%
>>
>> 2), about top thoughput: +39%
>>
>> So in my test, even for a quad-core VM, if the virtqueue number
>> is increased from 1 to 2, both scalability and performance can
>> get improved a lot.
>>
>> In above qemu implementation of virtio-blk-mq device, only one
>> IOthread handles requests from all vqs, and the above throughput
>> data has been very close to same fio test in host side with single
>> job. So more improvement should be observed once more IOthreads are
>> used for handling requests from multi vqs.
>>
>> TODO:
>> - adjust vq's irq smp_affinity according to blk-mq hw queue's cpumask
>>
>> V3:
>> - fix use-after-free on vq->name reported by Michael
>>
>> V2: (suggestions from Michael and Dave Chinner)
>> - allocate virtqueues' pointers dynamically
>> - make sure the per-queue spinlock isn't kept in same cache line
>> - make each queue's name different
>>
>> V1:
>> - remove RFC since no one objects
>> - add '__u8 unused' for pending as suggested by Rusty
>> - use virtio_cread_feature() directly, suggested by Rusty
>
> Sorry, please add Jens' reviewed-by.
>
> Reviewed-by: Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
I appreciate very much that one of you may queue these two
patches into your tree so that userspace work can be kicked off,
since Michael has acked both patches and all comments have
been addressed already.
Thanks,
--
Ming Lei
^ permalink raw reply
* Re: [PATCH v3 0/2] block: virtio-blk: support multi vq per virtio-blk
From: Jens Axboe @ 2014-07-01 3:01 UTC (permalink / raw)
To: Ming Lei, Linux Kernel Mailing List
Cc: Michael S. Tsirkin, linux-api, Linux Virtualization,
Stefan Hajnoczi, Paolo Bonzini
In-Reply-To: <CACVXFVMb4UXRYSp6nvjDQrDRcLP6rJ3_3QHYMzKv9MoogsU66w@mail.gmail.com>
On 2014-06-30 19:36, Ming Lei wrote:
> Hi Jens and Rusty,
>
> On Thu, Jun 26, 2014 at 8:04 PM, Ming Lei <ming.lei@canonical.com> wrote:
>> On Thu, Jun 26, 2014 at 5:41 PM, Ming Lei <ming.lei@canonical.com> wrote:
>>> Hi,
>>>
>>> These patches try to support multi virtual queues(multi-vq) in one
>>> virtio-blk device, and maps each virtual queue(vq) to blk-mq's
>>> hardware queue.
>>>
>>> With this approach, both scalability and performance on virtio-blk
>>> device can get improved.
>>>
>>> For verifying the improvement, I implements virtio-blk multi-vq over
>>> qemu's dataplane feature, and both handling host notification
>>> from each vq and processing host I/O are still kept in the per-device
>>> iothread context, the change is based on qemu v2.0.0 release, and
>>> can be accessed from below tree:
>>>
>>> git://kernel.ubuntu.com/ming/qemu.git #v2.0.0-virtblk-mq.1
>>>
>>> For enabling the multi-vq feature, 'num_queues=N' need to be added into
>>> '-device virtio-blk-pci ...' of qemu command line, and suggest to pass
>>> 'vectors=N+1' to keep one MSI irq vector per each vq, and the feature
>>> depends on x-data-plane.
>>>
>>> Fio(libaio, randread, iodepth=64, bs=4K, jobs=N) is run inside VM to
>>> verify the improvement.
>>>
>>> I just create a small quadcore VM and run fio inside the VM, and
>>> num_queues of the virtio-blk device is set as 2, but looks the
>>> improvement is still obvious. The host is 2 sockets, 8cores(16threads)
>>> server.
>>>
>>> 1), about scalability
>>> - jobs = 2, thoughput: +33%
>>> - jobs = 4, thoughput: +100%
>>>
>>> 2), about top thoughput: +39%
>>>
>>> So in my test, even for a quad-core VM, if the virtqueue number
>>> is increased from 1 to 2, both scalability and performance can
>>> get improved a lot.
>>>
>>> In above qemu implementation of virtio-blk-mq device, only one
>>> IOthread handles requests from all vqs, and the above throughput
>>> data has been very close to same fio test in host side with single
>>> job. So more improvement should be observed once more IOthreads are
>>> used for handling requests from multi vqs.
>>>
>>> TODO:
>>> - adjust vq's irq smp_affinity according to blk-mq hw queue's cpumask
>>>
>>> V3:
>>> - fix use-after-free on vq->name reported by Michael
>>>
>>> V2: (suggestions from Michael and Dave Chinner)
>>> - allocate virtqueues' pointers dynamically
>>> - make sure the per-queue spinlock isn't kept in same cache line
>>> - make each queue's name different
>>>
>>> V1:
>>> - remove RFC since no one objects
>>> - add '__u8 unused' for pending as suggested by Rusty
>>> - use virtio_cread_feature() directly, suggested by Rusty
>>
>> Sorry, please add Jens' reviewed-by.
>>
>> Reviewed-by: Jens Axboe <axboe@kernel.dk>
>
> I appreciate very much that one of you may queue these two
> patches into your tree so that userspace work can be kicked off,
> since Michael has acked both patches and all comments have
> been addressed already.
Given that Michael also acked it and Rusty is on his sabbatical, I'll
queue it up for 3.17.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH RFC net-next 03/14] bpf: introduce syscall(BPF, ...) and BPF maps
From: Alexei Starovoitov @ 2014-07-01 5:47 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
Network Development, linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrW3=idHOKF56d94suiA0NoiUGwr7pENm13q6=1XMbBPdw@mail.gmail.com>
On Mon, Jun 30, 2014 at 3:09 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Sat, Jun 28, 2014 at 11:36 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On Sat, Jun 28, 2014 at 6:52 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Sat, Jun 28, 2014 at 1:49 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>>
>>>> Sorry I don't like 'fd' direction at all.
>>>> 1. it will make the whole thing very socket specific and 'net' dependent.
>>>> but the goal here is to be able to use eBPF for tracing in embedded
>>>> setups. So it's gotta be net independent.
>>>> 2. sockets are already overloaded with all sorts of stuff. Adding more
>>>> types of sockets will complicate it a lot.
>>>> 3. and most important. read/write operations on sockets are not
>>>> done every nanosecond, whereas lookup operations on bpf maps
>>>> are done every dozen instructions, so we cannot have any overhead
>>>> when accessing maps.
>>>> In other words the verifier is done as static analyzer. I moved all
>>>> the complexity to verify time, so at run-time the programs are as
>>>> fast as possible. I'm strongly against run-time checks in critical path,
>>>> since they kill performance and make the whole approach a lot less usable.
>>>
>>> I may have described my suggestion poorly. I'm suggesting that all of
>>> these global ids be replaced *for userspace's benefit* with fds. That
>>> is, a map would have an associated struct inode, and, when you load an
>>> eBPF program, you'd pass fds into the kernel instead of global ids.
>>> The kernel would still compile the eBPF program to use the global ids,
>>> though.
>>
>> Hmm. If I understood you correctly, you're suggesting to do it similar
>> to ipc/mqueue, shmem, sockets do. By registering and mounting
>> a file system and providing all superblock and inode hooks… and
>> probably have its own namespace type… hmm… may be. That's
>> quite a bit of work to put lightly. As I said in the other email the first
>> step is root only and all these complexity just not worth doing
>> at this stage.
>
> The downside of not doing it right away is that it's harder to
> retrofit in without breaking early users.
>
> You might be able to get away with using anon_inodes. That will
Spent quite a bit of time playing with anon_inode_getfd(). The model
works ok for seccomp, but doesn't seem to work for tracing,
since tracepoints are global. Say, syscall(bpf, load_prog) returns
a process-local fd. This 'fd' as a string can be written to
debugfs/tracing/events/.../filter which will increment a refcnt of a global
ebpf_program structure and will keep using it. When process exits it will
close all fds which in case of ebpf_prog_fd should be a nop, since
the program is still attached to a global event. Now we have a
program and maps that still alive and dangling, since tracepoint events
keep coming, but no new process can access it. Here we just lost all
benefits of making it 'fd' based. Theoretically we can extend tracing to
be fd-based too and tracepoints will auto-detach upon process exit,
but that's not going to work for all other global events. Like networking
components (bridge, ovs, …) are global and they won't be adding
fd-based interfaces.
I'm still thinking about it, but it looks like that any process-local
ebpf_prog_id scheme is not going to work for global events. Thoughts?
^ permalink raw reply
* Re: [PATCH RFC net-next 00/14] BPF syscall, maps, verifier, samples
From: Daniel Borkmann @ 2014-07-01 7:18 UTC (permalink / raw)
To: Kees Cook
Cc: Alexei Starovoitov, David S. Miller, Ingo Molnar, Linus Torvalds,
Steven Rostedt, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Linux API, Network Development,
LKML
In-Reply-To: <CAGXu5jK9Bwocjz8y26=GEk0qg5ru1Mu7j9FVuu20KfTDUrSkuQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 07/01/2014 01:09 AM, Kees Cook wrote:
> On Fri, Jun 27, 2014 at 5:05 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>> Hi All,
>>
>> this patch set demonstrates the potential of eBPF.
>>
>> First patch "net: filter: split filter.c into two files" splits eBPF interpreter
>> out of networking into kernel/bpf/. The goal for BPF subsystem is to be usable
>> in NET-less configuration. Though the whole set is marked is RFC, the 1st patch
>> is good to go. Similar version of the patch that was posted few weeks ago, but
>> was deferred. I'm assuming due to lack of forward visibility. I hope that this
>> patch set shows what eBPF is capable of and where it's heading.
>>
>> Other patches expose eBPF instruction set to user space and introduce concepts
>> of maps and programs accessible via syscall.
>>
>> 'maps' is a generic storage of different types for sharing data between kernel
>> and userspace. Maps are referrenced by global id. Root can create multiple
>> maps of different types where key/value are opaque bytes of data. It's up to
>> user space and eBPF program to decide what they store in the maps.
>>
>> eBPF programs are similar to kernel modules. They live in global space and
>> have unique prog_id. Each program is a safe run-to-completion set of
>> instructions. eBPF verifier statically determines that the program terminates
>> and safe to execute. During verification the program takes a hold of maps
>> that it intends to use, so selected maps cannot be removed until program is
>> unloaded. The program can be attached to different events. These events can
>> be packets, tracepoint events and other types in the future. New event triggers
>> execution of the program which may store information about the event in the maps.
>> Beyond storing data the programs may call into in-kernel helper functions
>> which may, for example, dump stack, do trace_printk or other forms of live
>> kernel debugging. Same program can be attached to multiple events. Different
>> programs can access the same map:
>>
>> tracepoint tracepoint tracepoint sk_buff sk_buff
>> event A event B event C on eth0 on eth1
>> | | | | |
>> | | | | |
>> --> tracing <-- tracing socket socket
>> prog_1 prog_2 prog_3 prog_4
>> | | | |
>> |--- -----| |-------| map_3
>> map_1 map_2
>>
>> User space (via syscall) and eBPF programs access maps concurrently.
>>
>> Last two patches are sample code. 1st demonstrates stateful packet inspection.
>> It counts tcp and udp packets on eth0. Should be easy to see how this eBPF
>> framework can be used for network analytics.
>> 2nd sample does simple 'drop monitor'. It attaches to kfree_skb tracepoint
>> event and counts number of packet drops at particular $pc location.
>> User space periodically summarizes what eBPF programs recorded.
>> In these two samples the eBPF programs are tiny and written in 'assembler'
>> with macroses. More complex programs can be written C (llvm backend is not
>> part of this diff to reduce 'huge' perception).
>> Since eBPF is fully JITed on x64, the cost of running eBPF program is very
>> small even for high frequency events. Here are the numbers comparing
>> flow_dissector in C vs eBPF:
>> x86_64 skb_flow_dissect() same skb (all cached) - 42 nsec per call
>> x86_64 skb_flow_dissect() different skbs (cache misses) - 141 nsec per call
>> eBPF+jit skb_flow_dissect() same skb (all cached) - 51 nsec per call
>> eBPF+jit skb_flow_dissect() different skbs (cache misses) - 135 nsec per call
>>
>> Detailed explanation on eBPF verifier and safety is in patch 08/14
>
> This is very exciting! Thanks for working on it. :)
>
> Between the new eBPF syscall and the new seccomp syscall, I'm really
> looking forward to using lookup tables for seccomp filters. Under
> certain types of filters, we'll likely see some non-trivial
> performance improvements.
Well, if I read this correctly, the eBPF syscall lets you set up maps, etc,
but the only way to attach eBPF is via setsockopt for network filters right
now (and via tracing). Seccomp will still make use of classic BPF, so you
won't be able to use it there.
^ permalink raw reply
* Re: [PATCH RFC net-next 08/14] bpf: add eBPF verifier
From: Daniel Borkmann @ 2014-07-01 8:05 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403913966-4927-9-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On 06/28/2014 02:06 AM, Alexei Starovoitov wrote:
> Safety of eBPF programs is statically determined by the verifier, which detects:
> - loops
> - out of range jumps
> - unreachable instructions
> - invalid instructions
> - uninitialized register access
> - uninitialized stack access
> - misaligned stack access
> - out of range stack access
> - invalid calling convention
...
> More details in Documentation/networking/filter.txt
>
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
> ---
...
> kernel/bpf/verifier.c | 1431 +++++++++++++++++++++++++++++++++++
Looking at classic BPF verifier which checks safety of BPF
user space programs, it's roughly 200 loc. :-/
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> new file mode 100644
...
> +#define _(OP) ({ int ret = OP; if (ret < 0) return ret; })
...
> + _(get_map_info(env, map_id, &map));
...
> + _(size = bpf_size_to_bytes(bpf_size));
Nit: such macros should be removed, please.
^ permalink raw reply
* Re: [PATCH v3 0/2] block: virtio-blk: support multi vq per virtio-blk
From: Christoph Hellwig @ 2014-07-01 8:13 UTC (permalink / raw)
To: Jens Axboe
Cc: Michael S. Tsirkin, linux-api, Ming Lei,
Linux Kernel Mailing List, Linux Virtualization, Stefan Hajnoczi,
Paolo Bonzini
In-Reply-To: <53B22473.8010709@kernel.dk>
On Mon, Jun 30, 2014 at 09:01:07PM -0600, Jens Axboe wrote:
> >I appreciate very much that one of you may queue these two
> >patches into your tree so that userspace work can be kicked off,
> >since Michael has acked both patches and all comments have
> >been addressed already.
>
> Given that Michael also acked it and Rusty is on his sabbatical, I'll queue
> it up for 3.17.
So Rusty is offline? Who is taking care of module/moduleparam patches
in the meantime?
^ permalink raw reply
* Re: [PATCH RFC net-next 11/14] tracing: allow eBPF programs to be attached to events
From: Daniel Borkmann @ 2014-07-01 8:30 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403913966-4927-12-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On 06/28/2014 02:06 AM, Alexei Starovoitov wrote:
> User interface:
> cat bpf_123 > /sys/kernel/debug/tracing/__event__/filter
>
> where 123 is an id of the eBPF program priorly loaded.
> __event__ is static tracepoint event.
> (kprobe events will be supported in the future patches)
>
> eBPF programs can call in-kernel helper functions to:
> - lookup/update/delete elements in maps
> - memcmp
> - trace_printk
> - load_pointer
> - dump_stack
Are there plans to let eBPF replace the generic event
filtering framework in tracing?
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
> ---
> include/linux/ftrace_event.h | 5 +
> include/trace/bpf_trace.h | 29 +++++
> include/trace/ftrace.h | 10 ++
> include/uapi/linux/bpf.h | 5 +
> kernel/trace/Kconfig | 1 +
> kernel/trace/Makefile | 1 +
> kernel/trace/bpf_trace.c | 217 ++++++++++++++++++++++++++++++++++++
> kernel/trace/trace.h | 3 +
> kernel/trace/trace_events.c | 7 ++
> kernel/trace/trace_events_filter.c | 72 +++++++++++-
> 10 files changed, 349 insertions(+), 1 deletion(-)
> create mode 100644 include/trace/bpf_trace.h
> create mode 100644 kernel/trace/bpf_trace.c
^ permalink raw reply
* Re: [PATCH 5/5] man-pages: cap_rights_get: retrieve Capsicum fd rights
From: David Drysdale @ 2014-07-01 9:19 UTC (permalink / raw)
To: Andy Lutomirski
Cc: LSM List, linux-kernel@vger.kernel.org, Greg Kroah-Hartman,
Alexander Viro, Meredydd Luff, Kees Cook, James Morris, Linux API
In-Reply-To: <CALCETrW94NY+SNGsW3PYsdHfwpykPYs55FFDztQ2MVwMLmwJ4Q@mail.gmail.com>
On Mon, Jun 30, 2014 at 03:28:14PM -0700, Andy Lutomirski wrote:
> On Mon, Jun 30, 2014 at 3:28 AM, David Drysdale <drysdale@google.com> wrote:
> > Signed-off-by: David Drysdale <drysdale@google.com>
> > ---
> > man2/cap_rights_get.2 | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 126 insertions(+)
> > create mode 100644 man2/cap_rights_get.2
> >
> > diff --git a/man2/cap_rights_get.2 b/man2/cap_rights_get.2
> > new file mode 100644
> > index 000000000000..966c0ed7e336
> > --- /dev/null
> > +++ b/man2/cap_rights_get.2
> > @@ -0,0 +1,126 @@
> > +.\"
> > +.\" Copyright (c) 2008-2010 Robert N. M. Watson
> > +.\" Copyright (c) 2012-2013 The FreeBSD Foundation
> > +.\" Copyright (c) 2013-2014 Google, Inc.
> > +.\" All rights reserved.
> > +.\"
> > +.\" %%%LICENSE_START(BSD_2_CLAUSE)
> > +.\" Redistribution and use in source and binary forms, with or without
> > +.\" modification, are permitted provided that the following conditions
> > +.\" are met:
> > +.\" 1. Redistributions of source code must retain the above copyright
> > +.\" notice, this list of conditions and the following disclaimer.
> > +.\" 2. Redistributions in binary form must reproduce the above copyright
> > +.\" notice, this list of conditions and the following disclaimer in the
> > +.\" documentation and/or other materials provided with the distribution.
> > +.\"
> > +.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > +.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > +.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > +.\" ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> > +.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > +.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > +.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > +.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > +.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > +.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > +.\" SUCH DAMAGE.
> > +.\" %%%LICENSE_END
> > +.\"
> > +.TH CAP_RIGHTS_GET 2 2014-05-07 "Linux" "Linux Programmer's Manual"
> > +.SH NAME
> > +cap_rights_get \- retrieve Capsicum capability rights
> > +.SH SYNOPSIS
> > +.nf
> > +.B #include <sys/capsicum.h>
> > +.sp
> > +.BI "int cap_rights_get(int " fd ", struct cap_rights *" rights ,
> > +.BI " unsigned int *" fcntls ,
> > +.BI " int *" nioctls ", unsigned int *" ioctls );
> > +.SH DESCRIPTION
> > +Obtain the current Capsicum capability rights for a file descriptor.
> > +.PP
> > +The function will fill the
> > +.I rights
> > +argument (if non-NULL) with the primary capability rights of the
> > +.I fd
> > +descriptor. The result can be examined with the
> > +.BR cap_rights_is_set (3)
> > +family of functions. The complete list of primary rights can be found in the
> > +.BR rights (7)
> > +manual page.
> > +.PP
> > +If the
> > +.I fcntls
> > +argument is non-NULL, it will be filled in with a bitmask of allowed
> > +.BR fcntl (2)
> > +commands; see
> > +.BR cap_rights_limit (2)
> > +for values. If the file descriptor does not have the
> > +.B CAP_FCNTL
> > +primary right, the returned
> > +.I fcntls
> > +value will be zero.
> > +.PP
> > +If the
> > +.I nioctls
> > +argument is non-NULL, it will be filled in with the number of allowed
> > +.BR ioctl (2)
> > +commands, or with the value CAP_IOCTLS_ALL to indicate that all
> > +.BR ioctl (2)
> > +commands are allowed. If the file descriptor does not have the
> > +.B CAP_IOCTL
> > +primary right, the returned
> > +.I nioctls
> > +value will be zero.
> > +.PP
> > +The
> > +.I ioctls
> > +argument (if non-NULL) should point at memory that can hold up to
> > +.I nioctls
> > +values.
> > +The system call populates the provided buffer with up to
> > +.I nioctls
> > +elements, but always returns the total number of
>
> I assume you mean "up to the initial value of *nioctls elements" or
> something. Can you clarify?
>
> --Andy
Yeah, that's what I meant. Is this clearer?
If the ioctls argument is non-NULL, the caller should specify
the size of the provided buffer as the initial value of the
nioctls argument (as a count of the number of ioctl(2) command
values the buffer can hold). On successful completion of the
system call, the ioctls buffer is filled with the ioctl(2) com‐
mand values, up to maximum of the initial value of nioctls.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 01/11] fs: add O_BENEATH_ONLY flag to openat(2)
From: David Drysdale @ 2014-07-01 9:53 UTC (permalink / raw)
To: Andi Kleen
Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87mwcuw2pj.fsf-KWJ+5VKanrL29G5dvP0v1laTQe2KTcn/@public.gmane.org>
On Mon, Jun 30, 2014 at 01:40:40PM -0700, Andi Kleen wrote:
> David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> writes:
>
> > Add a new O_BENEATH_ONLY flag for openat(2) which restricts the
> > provided path, rejecting (with -EACCES) paths that are not beneath
> > the provided dfd. In particular, reject:
> > - paths that contain .. components
> > - paths that begin with /
> > - symlinks that have paths as above.
>
> How about bind mounts?
>
> -Andi
>
> --
> ak-VuQAYsv1563Yd54FQh9/CA@public.gmane.org -- Speaking for myself only
Bind mounts won't get rejected because they just look like normal
path components. In other words, if dir/subdir is a bind mount to
/root/dir then:
fd = openat(AT_FDCWD, "dir/subdir", O_RDONLY|O_BENEATH_ONLY);
will work fine.
^ permalink raw reply
* Re: [PATCH v9] mm: support madvise(MADV_FREE)
From: Rik van Riel @ 2014-07-01 14:16 UTC (permalink / raw)
To: Minchan Kim, Andrew Morton
Cc: linux-kernel, linux-mm, Michael Kerrisk, Linux API, Hugh Dickins,
Johannes Weiner, KOSAKI Motohiro, Mel Gorman, Jason Evans,
Zhang Yanfei
In-Reply-To: <1404174975-22019-1-git-send-email-minchan@kernel.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/30/2014 08:36 PM, Minchan Kim wrote:
> Linux doesn't have an ability to free pages lazy while other OS
> already have been supported that named by madvise(MADV_FREE).
>
> The gain is clear that kernel can discard freed pages rather than
> swapping out or OOM if memory pressure happens.
>
> Without memory pressure, freed pages would be reused by userspace
> without another additional overhead(ex, page fault + allocation +
> zeroing).
> Cc: Michael Kerrisk <mtk.manpages@gmail.com> Cc: Linux API
> <linux-api@vger.kernel.org> Cc: Hugh Dickins <hughd@google.com> Cc:
> Johannes Weiner <hannes@cmpxchg.org> Cc: Rik van Riel
> <riel@redhat.com> Cc: KOSAKI Motohiro
> <kosaki.motohiro@jp.fujitsu.com> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Jason Evans <je@fb.com> Cc: Zhang Yanfei
> <zhangyanfei@cn.fujitsu.com> Signed-off-by: Minchan Kim
> <minchan@kernel.org>
Acked-by: Rik van Riel <riel@redhat.com>
- --
All rights reversed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJTssKpAAoJEM553pKExN6DspUH/3fdn95zVIA6GGfmFG/g05Fm
SYv82v0ee2gGM7yRGeVkFSVuj5qYCneyJeprERHBs43huafqqnWd9MMcZxxskNk7
MpyVmRsCh54qC2Y6Rqu5E15jEKjCcxss1vCbHp0ExtZHnfU29re+JB0oRE9IKszW
p2r6rsolHtNY4otTAQ6pAtA6ioH1E0xppK5mpqHAUpFJuq3PqXbSsptFdl6AJciw
25zBB6iOdVgpciYwkn7yBvaZiY+sRuiRFSAH0klQVHlX0ZueIXYnJtybVhHSqGs/
Nu1/zhrRrohOcj0Ka6cTJBBH2RyXTmgcurfTUlI4IZzcDqJWtuXjXBty0wkhIZQ=
=RjYx
-----END PGP SIGNATURE-----
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 5/5] man-pages: cap_rights_get: retrieve Capsicum fd rights
From: Andy Lutomirski @ 2014-07-01 14:18 UTC (permalink / raw)
To: David Drysdale
Cc: LSM List, linux-kernel@vger.kernel.org, Greg Kroah-Hartman,
Alexander Viro, Meredydd Luff, Kees Cook, James Morris, Linux API
In-Reply-To: <20140701091900.GB2242@google.com>
On Tue, Jul 1, 2014 at 2:19 AM, David Drysdale <drysdale@google.com> wrote:
> On Mon, Jun 30, 2014 at 03:28:14PM -0700, Andy Lutomirski wrote:
>> On Mon, Jun 30, 2014 at 3:28 AM, David Drysdale <drysdale@google.com> wrote:
>> > Signed-off-by: David Drysdale <drysdale@google.com>
>> > ---
>> > man2/cap_rights_get.2 | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 126 insertions(+)
>> > create mode 100644 man2/cap_rights_get.2
>> >
>> > diff --git a/man2/cap_rights_get.2 b/man2/cap_rights_get.2
>> > new file mode 100644
>> > index 000000000000..966c0ed7e336
>> > --- /dev/null
>> > +++ b/man2/cap_rights_get.2
>> > @@ -0,0 +1,126 @@
>> > +.\"
>> > +.\" Copyright (c) 2008-2010 Robert N. M. Watson
>> > +.\" Copyright (c) 2012-2013 The FreeBSD Foundation
>> > +.\" Copyright (c) 2013-2014 Google, Inc.
>> > +.\" All rights reserved.
>> > +.\"
>> > +.\" %%%LICENSE_START(BSD_2_CLAUSE)
>> > +.\" Redistribution and use in source and binary forms, with or without
>> > +.\" modification, are permitted provided that the following conditions
>> > +.\" are met:
>> > +.\" 1. Redistributions of source code must retain the above copyright
>> > +.\" notice, this list of conditions and the following disclaimer.
>> > +.\" 2. Redistributions in binary form must reproduce the above copyright
>> > +.\" notice, this list of conditions and the following disclaimer in the
>> > +.\" documentation and/or other materials provided with the distribution.
>> > +.\"
>> > +.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
>> > +.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> > +.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>> > +.\" ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
>> > +.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>> > +.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
>> > +.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>> > +.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
>> > +.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
>> > +.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>> > +.\" SUCH DAMAGE.
>> > +.\" %%%LICENSE_END
>> > +.\"
>> > +.TH CAP_RIGHTS_GET 2 2014-05-07 "Linux" "Linux Programmer's Manual"
>> > +.SH NAME
>> > +cap_rights_get \- retrieve Capsicum capability rights
>> > +.SH SYNOPSIS
>> > +.nf
>> > +.B #include <sys/capsicum.h>
>> > +.sp
>> > +.BI "int cap_rights_get(int " fd ", struct cap_rights *" rights ,
>> > +.BI " unsigned int *" fcntls ,
>> > +.BI " int *" nioctls ", unsigned int *" ioctls );
>> > +.SH DESCRIPTION
>> > +Obtain the current Capsicum capability rights for a file descriptor.
>> > +.PP
>> > +The function will fill the
>> > +.I rights
>> > +argument (if non-NULL) with the primary capability rights of the
>> > +.I fd
>> > +descriptor. The result can be examined with the
>> > +.BR cap_rights_is_set (3)
>> > +family of functions. The complete list of primary rights can be found in the
>> > +.BR rights (7)
>> > +manual page.
>> > +.PP
>> > +If the
>> > +.I fcntls
>> > +argument is non-NULL, it will be filled in with a bitmask of allowed
>> > +.BR fcntl (2)
>> > +commands; see
>> > +.BR cap_rights_limit (2)
>> > +for values. If the file descriptor does not have the
>> > +.B CAP_FCNTL
>> > +primary right, the returned
>> > +.I fcntls
>> > +value will be zero.
>> > +.PP
>> > +If the
>> > +.I nioctls
>> > +argument is non-NULL, it will be filled in with the number of allowed
>> > +.BR ioctl (2)
>> > +commands, or with the value CAP_IOCTLS_ALL to indicate that all
>> > +.BR ioctl (2)
>> > +commands are allowed. If the file descriptor does not have the
>> > +.B CAP_IOCTL
>> > +primary right, the returned
>> > +.I nioctls
>> > +value will be zero.
>> > +.PP
>> > +The
>> > +.I ioctls
>> > +argument (if non-NULL) should point at memory that can hold up to
>> > +.I nioctls
>> > +values.
>> > +The system call populates the provided buffer with up to
>> > +.I nioctls
>> > +elements, but always returns the total number of
>>
>> I assume you mean "up to the initial value of *nioctls elements" or
>> something. Can you clarify?
>>
>> --Andy
>
> Yeah, that's what I meant. Is this clearer?
>
> If the ioctls argument is non-NULL, the caller should specify
> the size of the provided buffer as the initial value of the
> nioctls argument (as a count of the number of ioctl(2) command
> values the buffer can hold). On successful completion of the
> system call, the ioctls buffer is filled with the ioctl(2) com‐
> mand values, up to maximum of the initial value of nioctls.
>
Yes. Thanks.
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH v9] mm: support madvise(MADV_FREE)
From: Kirill A. Shutemov @ 2014-07-01 14:50 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, linux-kernel, linux-mm, Michael Kerrisk, Linux API,
Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
Mel Gorman, Jason Evans, Zhang Yanfei
In-Reply-To: <1404174975-22019-1-git-send-email-minchan@kernel.org>
On Tue, Jul 01, 2014 at 09:36:15AM +0900, Minchan Kim wrote:
> + do {
> + /*
> + * XXX: We can optimize with supporting Hugepage free
> + * if the range covers.
> + */
> + next = pmd_addr_end(addr, end);
> + if (pmd_trans_huge(*pmd))
> + split_huge_page_pmd(vma, addr, pmd);
Could you implement proper THP support before upstreaming the feature?
It shouldn't be a big deal.
> + /*
> + * Here there can be other concurrent MADV_DONTNEED or
> + * trans huge page faults running, and if the pmd is
> + * none or trans huge it can change under us. This is
> + * because MADV_LAZYFREE holds the mmap_sem in read
> + * mode.
> + */
> + if (pmd_none_or_trans_huge_or_clear_bad(pmd))
> + goto next;
> + next = madvise_free_pte_range(tlb, vma, pmd, addr, next);
> +next:
> + cond_resched();
> + } while (pmd++, addr = next, addr != end);
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH RFC net-next 03/14] bpf: introduce syscall(BPF, ...) and BPF maps
From: Andy Lutomirski @ 2014-07-01 15:11 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMEtUuyX-tybpMEW=f-00qgq9h3AcHovLNW0_bak3oT4Oj3FuA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Jun 30, 2014 at 10:47 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Mon, Jun 30, 2014 at 3:09 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Sat, Jun 28, 2014 at 11:36 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>> On Sat, Jun 28, 2014 at 6:52 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Sat, Jun 28, 2014 at 1:49 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>>>
>>>>> Sorry I don't like 'fd' direction at all.
>>>>> 1. it will make the whole thing very socket specific and 'net' dependent.
>>>>> but the goal here is to be able to use eBPF for tracing in embedded
>>>>> setups. So it's gotta be net independent.
>>>>> 2. sockets are already overloaded with all sorts of stuff. Adding more
>>>>> types of sockets will complicate it a lot.
>>>>> 3. and most important. read/write operations on sockets are not
>>>>> done every nanosecond, whereas lookup operations on bpf maps
>>>>> are done every dozen instructions, so we cannot have any overhead
>>>>> when accessing maps.
>>>>> In other words the verifier is done as static analyzer. I moved all
>>>>> the complexity to verify time, so at run-time the programs are as
>>>>> fast as possible. I'm strongly against run-time checks in critical path,
>>>>> since they kill performance and make the whole approach a lot less usable.
>>>>
>>>> I may have described my suggestion poorly. I'm suggesting that all of
>>>> these global ids be replaced *for userspace's benefit* with fds. That
>>>> is, a map would have an associated struct inode, and, when you load an
>>>> eBPF program, you'd pass fds into the kernel instead of global ids.
>>>> The kernel would still compile the eBPF program to use the global ids,
>>>> though.
>>>
>>> Hmm. If I understood you correctly, you're suggesting to do it similar
>>> to ipc/mqueue, shmem, sockets do. By registering and mounting
>>> a file system and providing all superblock and inode hooks… and
>>> probably have its own namespace type… hmm… may be. That's
>>> quite a bit of work to put lightly. As I said in the other email the first
>>> step is root only and all these complexity just not worth doing
>>> at this stage.
>>
>> The downside of not doing it right away is that it's harder to
>> retrofit in without breaking early users.
>>
>> You might be able to get away with using anon_inodes. That will
>
> Spent quite a bit of time playing with anon_inode_getfd(). The model
> works ok for seccomp, but doesn't seem to work for tracing,
> since tracepoints are global. Say, syscall(bpf, load_prog) returns
> a process-local fd. This 'fd' as a string can be written to
> debugfs/tracing/events/.../filter which will increment a refcnt of a global
> ebpf_program structure and will keep using it. When process exits it will
> close all fds which in case of ebpf_prog_fd should be a nop, since
> the program is still attached to a global event. Now we have a
> program and maps that still alive and dangling, since tracepoint events
> keep coming, but no new process can access it. Here we just lost all
> benefits of making it 'fd' based. Theoretically we can extend tracing to
> be fd-based too and tracepoints will auto-detach upon process exit,
> but that's not going to work for all other global events. Like networking
> components (bridge, ovs, …) are global and they won't be adding
> fd-based interfaces.
> I'm still thinking about it, but it looks like that any process-local
> ebpf_prog_id scheme is not going to work for global events. Thoughts?
Hmm. Maybe these things do need global ids for tracing, or at least
there need to be some way to stash them somewhere and find them again.
I suppose that debugfs could have symlinks to them, but I don't know
how hard that would be to implement or how awkward it would be to use.
I imagine there's some awkwardness regardless. For tracing, if I
create map 75 and eBPF program 492 that uses map 75, then I still need
to remember that map 75 is the map I want (or I need to parse the eBPF
program later on).
How do you imagine the userspace code working? Maybe it would make
sense to add some nlattrs for eBPF programs to map between referenced
objects and nicknames for them. Then user code could look at
/sys/kernel/debug/whatever/nickname_of_map to resolve the map id or
even just open it directly.
I admit that I'm much more familiar with seccomp and even socket
filters than I am with tracing.
--Andy
^ permalink raw reply
* Re: [PATCH 01/11] fs: add O_BENEATH_ONLY flag to openat(2)
From: Loganaden Velvindron @ 2014-07-01 18:58 UTC (permalink / raw)
To: David Drysdale
Cc: Andi Kleen, linux-security-module, linux-kernel,
Greg Kroah-Hartman, Alexander Viro, Meredydd Luff, Kees Cook,
James Morris, linux-api
In-Reply-To: <20140701095356.GC2242@google.com>
On Tue, Jul 1, 2014 at 1:53 PM, David Drysdale <drysdale@google.com> wrote:
> On Mon, Jun 30, 2014 at 01:40:40PM -0700, Andi Kleen wrote:
>> David Drysdale <drysdale@google.com> writes:
>>
>> > Add a new O_BENEATH_ONLY flag for openat(2) which restricts the
>> > provided path, rejecting (with -EACCES) paths that are not beneath
>> > the provided dfd. In particular, reject:
>> > - paths that contain .. components
>> > - paths that begin with /
>> > - symlinks that have paths as above.
>>
>> How about bind mounts?
>>
>> -Andi
>>
>> --
>> ak@linux.intel.com -- Speaking for myself only
>
> Bind mounts won't get rejected because they just look like normal
> path components. In other words, if dir/subdir is a bind mount to
> /root/dir then:
> fd = openat(AT_FDCWD, "dir/subdir", O_RDONLY|O_BENEATH_ONLY);
> will work fine.
Talking about David's efforts at porting Capsicum to Linux, I've
already implemented
support for Capsicum in OpenSSH. It shouldn't be complicated to enable
it on Linux
systems that support it.
I would very like to see capsicum integrated into mainline, as it's a
high quality sandbox
solution, that will benefit a lot of server software that implement
privilege separation.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
This message is strictly personal and the opinions expressed do not
represent those of my employers, either past or present.
^ permalink raw reply
* Re: [PATCH RFC net-next 08/14] bpf: add eBPF verifier
From: Alexei Starovoitov @ 2014-07-01 20:04 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
Network Development, LKML
In-Reply-To: <53B26BB0.90209-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Jul 1, 2014 at 1:05 AM, Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 06/28/2014 02:06 AM, Alexei Starovoitov wrote:
>>
>> Safety of eBPF programs is statically determined by the verifier, which
>> detects:
>> - loops
>> - out of range jumps
>> - unreachable instructions
>> - invalid instructions
>> - uninitialized register access
>> - uninitialized stack access
>> - misaligned stack access
>> - out of range stack access
>> - invalid calling convention
>
> ...
>
>> More details in Documentation/networking/filter.txt
>>
>> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
>> ---
>
> ...
>>
>> kernel/bpf/verifier.c | 1431
>> +++++++++++++++++++++++++++++++++++
>
>
> Looking at classic BPF verifier which checks safety of BPF
> user space programs, it's roughly 200 loc. :-/
I'm not sure what's your point comparing apples to oranges.
For the record 1431 lines include ~200 lines worth of comments
and 200 lines of verbose prints. Without them rejected eBPF
program is black box. Users need a way to understand why
verifier rejected it.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> new file mode 100644
>
> ...
>
>> +#define _(OP) ({ int ret = OP; if (ret < 0) return ret; })
>
> ...
>>
>> + _(get_map_info(env, map_id, &map));
>
> ...
>>
>> + _(size = bpf_size_to_bytes(bpf_size));
>
>
> Nit: such macros should be removed, please.
It may surely look unconventional, but alternative is to replace
every usage of _ macro with:
err = …
if (err)
return err;
and since this macro is used 38 times, it will add ~120 unnecessary
lines that will only make code much harder to follow.
I tried not using macro and results were not pleasing.
^ permalink raw reply
* Re: [PATCH RFC net-next 11/14] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-07-01 20:06 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
Network Development, LKML
In-Reply-To: <53B271C0.5090008-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Jul 1, 2014 at 1:30 AM, Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 06/28/2014 02:06 AM, Alexei Starovoitov wrote:
>>
>> User interface:
>> cat bpf_123 > /sys/kernel/debug/tracing/__event__/filter
>>
>> where 123 is an id of the eBPF program priorly loaded.
>> __event__ is static tracepoint event.
>> (kprobe events will be supported in the future patches)
>>
>> eBPF programs can call in-kernel helper functions to:
>> - lookup/update/delete elements in maps
>> - memcmp
>> - trace_printk
>> - load_pointer
>> - dump_stack
>
>
> Are there plans to let eBPF replace the generic event
> filtering framework in tracing?
yes. the other patch that replaces predicate tree walking with
eBPF programs is pending on eBPF split out of networking.
^ permalink raw reply
* Re: Does it make sense to define a constant for openat and such that is guaranteed not to be used for special purposes later on?
From: Steven Stewart-Gallus @ 2014-07-02 2:19 UTC (permalink / raw)
To: Andy Lutomirski; +Cc: Linux API
In-Reply-To: <CALCETrUp=UYKEnhN9qf2vYToCV7YsLfL3+UGS-p9aY=_zMQMJA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
> I like this. Want to submit a patch?
>
> --Andy
>
Sure I'd love to patch the documentation. So I can just submit a man pages patch
for this and I don't have to coordinate with GLibc (or other liibcs like Musl)
for this or modify any in kernel tree documentation?
^ permalink raw reply
* Re: [PATCH RFC net-next 01/14] net: filter: split filter.c into two files
From: Namhyung Kim @ 2014-07-02 4:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403913966-4927-2-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Hi Alexei,
On Fri, 27 Jun 2014 17:05:53 -0700, Alexei Starovoitov wrote:
> BPF is used in several kernel components. This split creates logical boundary
> between generic eBPF core and the rest
>
> kernel/bpf/core.c: eBPF interpreter
>
> net/core/filter.c: classic->eBPF converter, classic verifiers, socket filters
>
> This patch only moves functions.
>
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
> ---
> kernel/Makefile | 1 +
> kernel/bpf/Makefile | 1 +
> kernel/bpf/core.c | 545 +++++++++++++++++++++++++++++++++++++++++++++++++++
> net/core/filter.c | 520 ------------------------------------------------
> 4 files changed, 547 insertions(+), 520 deletions(-)
> create mode 100644 kernel/bpf/Makefile
> create mode 100644 kernel/bpf/core.c
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> index f2a8b6246ce9..e7360b7c2c0e 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -87,6 +87,7 @@ obj-$(CONFIG_RING_BUFFER) += trace/
> obj-$(CONFIG_TRACEPOINTS) += trace/
> obj-$(CONFIG_IRQ_WORK) += irq_work.o
> obj-$(CONFIG_CPU_PM) += cpu_pm.o
> +obj-$(CONFIG_NET) += bpf/
But this still requires CONFIG_NET to use bpf. Why not adding
CONFIG_BPF and making CONFIG_NET selects it?
Thanks,
Namhyung
^ permalink raw reply
* Re: [PATCH RFC net-next 08/14] bpf: add eBPF verifier
From: Namhyung Kim @ 2014-07-02 5:05 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403913966-4927-9-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Mostly questions and few nitpicks.. :)
On Fri, 27 Jun 2014 17:06:00 -0700, Alexei Starovoitov wrote:
> +/* types of values:
> + * - stored in an eBPF register
> + * - passed into helper functions as an argument
> + * - returned from helper functions
> + */
> +enum bpf_reg_type {
> + INVALID_PTR, /* reg doesn't contain a valid pointer */
I don't think it's a good name. The INVALID_PTR can be read as it
contains a "pointer" which is invalid. Maybe INTEGER, NUMBER or
something different can be used. And I think the struct reg_state->ptr
should be renamed also.
> + PTR_TO_CTX, /* reg points to bpf_context */
> + PTR_TO_MAP, /* reg points to map element value */
> + PTR_TO_MAP_CONDITIONAL, /* points to map element value or NULL */
> + PTR_TO_STACK, /* reg == frame_pointer */
> + PTR_TO_STACK_IMM, /* reg == frame_pointer + imm */
> + PTR_TO_STACK_IMM_MAP_KEY, /* pointer to stack used as map key */
> + PTR_TO_STACK_IMM_MAP_VALUE, /* pointer to stack used as map elem */
So, these PTR_TO_STACK_IMM[_*] types are only for function argument,
right? I guessed it could be used to access memory in general too, but
then I thought it'd make verification complicated..
And I also agree that it'd better splitting reg types and function
argument constraints.
> + RET_INTEGER, /* function returns integer */
> + RET_VOID, /* function returns void */
> + CONST_ARG, /* function expects integer constant argument */
> + CONST_ARG_MAP_ID, /* int const argument that is used as map_id */
That means a map id should always be a constant (for verification), right?
> + /* int const argument indicating number of bytes accessed from stack
> + * previous function argument must be ptr_to_stack_imm
> + */
> + CONST_ARG_STACK_IMM_SIZE,
> +};
[SNIP]
> +
> +/* check read/write into map element returned by bpf_table_lookup() */
> +static int check_table_access(struct verifier_env *env, int regno, int off,
> + int size)
I guess the "table" is an old name of the "map"?
> +{
> + struct bpf_map *map;
> + int map_id = env->cur_state.regs[regno].imm;
> +
> + _(get_map_info(env, map_id, &map));
> +
> + if (off < 0 || off + size > map->value_size) {
> + verbose("invalid access to map_id=%d leaf_size=%d off=%d size=%d\n",
> + map_id, map->value_size, off, size);
> + return -EACCES;
> + }
> + return 0;
> +}
[SNIP]
> +static int check_mem_access(struct verifier_env *env, int regno, int off,
> + int bpf_size, enum bpf_access_type t,
> + int value_regno)
> +{
> + struct verifier_state *state = &env->cur_state;
> + int size;
> +
> + _(size = bpf_size_to_bytes(bpf_size));
> +
> + if (off % size != 0) {
> + verbose("misaligned access off %d size %d\n", off, size);
> + return -EACCES;
> + }
> +
> + if (state->regs[regno].ptr == PTR_TO_MAP) {
> + _(check_table_access(env, regno, off, size));
> + if (t == BPF_READ)
> + mark_reg_no_ptr(state->regs, value_regno);
> + } else if (state->regs[regno].ptr == PTR_TO_CTX) {
> + _(check_ctx_access(env, off, size, t));
> + if (t == BPF_READ)
> + mark_reg_no_ptr(state->regs, value_regno);
> + } else if (state->regs[regno].ptr == PTR_TO_STACK) {
> + if (off >= 0 || off < -MAX_BPF_STACK) {
> + verbose("invalid stack off=%d size=%d\n", off, size);
> + return -EACCES;
> + }
So memory (stack) access is only allowed for a stack base regsiter and a
constant offset, right?
> + if (t == BPF_WRITE)
> + _(check_stack_write(state, off, size, value_regno));
> + else
> + _(check_stack_read(state, off, size, value_regno));
> + } else {
> + verbose("R%d invalid mem access '%s'\n",
> + regno, reg_type_str[state->regs[regno].ptr]);
> + return -EACCES;
> + }
> + return 0;
> +}
[SNIP]
> +static int check_call(struct verifier_env *env, int func_id)
> +{
> + struct verifier_state *state = &env->cur_state;
> + const struct bpf_func_proto *fn = NULL;
> + struct reg_state *regs = state->regs;
> + struct bpf_map *map = NULL;
> + struct reg_state *reg;
> + int map_id = -1;
> + int i;
> +
> + /* find function prototype */
> + if (func_id <= 0 || func_id >= __BPF_FUNC_MAX_ID) {
> + verbose("invalid func %d\n", func_id);
> + return -EINVAL;
> + }
> +
> + if (env->prog->info->ops->get_func_proto)
> + fn = env->prog->info->ops->get_func_proto(func_id);
> +
> + if (!fn || (fn->ret_type != RET_INTEGER &&
> + fn->ret_type != PTR_TO_MAP_CONDITIONAL &&
> + fn->ret_type != RET_VOID)) {
> + verbose("unknown func %d\n", func_id);
> + return -EINVAL;
> + }
> +
> + /* check args */
> + _(check_func_arg(env, BPF_REG_1, fn->arg1_type, &map_id, &map));
> + _(check_func_arg(env, BPF_REG_2, fn->arg2_type, &map_id, &map));
> + _(check_func_arg(env, BPF_REG_3, fn->arg3_type, &map_id, &map));
> + _(check_func_arg(env, BPF_REG_4, fn->arg4_type, &map_id, &map));
Missing BPF_REG_5?
> +
> + /* reset caller saved regs */
> + for (i = 0; i < CALLER_SAVED_REGS; i++) {
> + reg = regs + caller_saved[i];
> + reg->read_ok = false;
> + reg->ptr = INVALID_PTR;
> + reg->imm = 0xbadbad;
> + }
> +
> + /* update return register */
> + reg = regs + BPF_REG_0;
> + if (fn->ret_type == RET_INTEGER) {
> + reg->read_ok = true;
> + reg->ptr = INVALID_PTR;
> + } else if (fn->ret_type != RET_VOID) {
> + reg->read_ok = true;
> + reg->ptr = fn->ret_type;
> + if (fn->ret_type == PTR_TO_MAP_CONDITIONAL)
> + /*
> + * remember map_id, so that check_table_access()
> + * can check 'value_size' boundary of memory access
> + * to map element returned from bpf_table_lookup()
> + */
> + reg->imm = map_id;
> + }
> + return 0;
> +}
[SNIP]
> +#define PEAK_INT() \
s/PEAK/PEEK/ ?
Thanks,
Namhyung
> + ({ \
> + int _ret; \
> + if (cur_stack == 0) \
> + _ret = -1; \
> + else \
> + _ret = stack[cur_stack - 1]; \
> + _ret; \
> + })
> +
> +#define POP_INT() \
> + ({ \
> + int _ret; \
> + if (cur_stack == 0) \
> + _ret = -1; \
> + else \
> + _ret = stack[--cur_stack]; \
> + _ret; \
> + })
^ permalink raw reply
* Re: [PATCH RFC net-next 11/14] tracing: allow eBPF programs to be attached to events
From: Namhyung Kim @ 2014-07-02 5:32 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403913966-4927-12-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On Fri, 27 Jun 2014 17:06:03 -0700, Alexei Starovoitov wrote:
> User interface:
> cat bpf_123 > /sys/kernel/debug/tracing/__event__/filter
>
> where 123 is an id of the eBPF program priorly loaded.
> __event__ is static tracepoint event.
> (kprobe events will be supported in the future patches)
>
> eBPF programs can call in-kernel helper functions to:
> - lookup/update/delete elements in maps
> - memcmp
> - trace_printk
ISTR Steve doesn't like to use trace_printk() (at least for production
kernels) anymore. And I'm not sure it'd work if there's no existing
trace_printk() on a system.
> - load_pointer
> - dump_stack
[SNIP]
> @@ -634,6 +635,15 @@ ftrace_raw_event_##call(void *__data, proto) \
> if (ftrace_trigger_soft_disabled(ftrace_file)) \
> return; \
> \
> + if (unlikely(ftrace_file->flags & FTRACE_EVENT_FL_FILTERED) && \
> + unlikely(ftrace_file->event_call->flags & TRACE_EVENT_FL_BPF)) { \
> + struct bpf_context __ctx; \
> + \
> + populate_bpf_context(&__ctx, args, 0, 0, 0, 0, 0); \
> + trace_filter_call_bpf(ftrace_file->filter, &__ctx); \
> + return; \
> + } \
> + \
Hmm.. But it seems the eBPF prog is not a filter - it'd always drop the
event. And I think it's better to use a recorded entry rather then args
as a bpf_context so that tools like perf can manipulate it at compile
time based on the event format.
Thanks,
Namhyung
> __data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
> \
> entry = ftrace_event_buffer_reserve(&fbuffer, ftrace_file, \
^ permalink raw reply
* Re: [PATCH RFC net-next 03/14] bpf: introduce syscall(BPF, ...) and BPF maps
From: Alexei Starovoitov @ 2014-07-02 5:33 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrWpA5M74pKJLFJ0t-2hi2TXMi_BV6DbJMmdDOJyOoHOyg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Jul 1, 2014 at 8:11 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Mon, Jun 30, 2014 at 10:47 PM, Alexei Starovoitov <ast-uqk4Ao+rVK7QFizaE/u3fw@public.gmane.orgm> wrote:
>> On Mon, Jun 30, 2014 at 3:09 PM, Andy Lutomirski <luto-kltTT9wpgjKXcx/E+B78Qg@public.gmane.orgt> wrote:
>>> On Sat, Jun 28, 2014 at 11:36 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>> On Sat, Jun 28, 2014 at 6:52 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>> On Sat, Jun 28, 2014 at 1:49 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>>>>
>>>>>> Sorry I don't like 'fd' direction at all.
>>>>>> 1. it will make the whole thing very socket specific and 'net' dependent.
>>>>>> but the goal here is to be able to use eBPF for tracing in embedded
>>>>>> setups. So it's gotta be net independent.
>>>>>> 2. sockets are already overloaded with all sorts of stuff. Adding more
>>>>>> types of sockets will complicate it a lot.
>>>>>> 3. and most important. read/write operations on sockets are not
>>>>>> done every nanosecond, whereas lookup operations on bpf maps
>>>>>> are done every dozen instructions, so we cannot have any overhead
>>>>>> when accessing maps.
>>>>>> In other words the verifier is done as static analyzer. I moved all
>>>>>> the complexity to verify time, so at run-time the programs are as
>>>>>> fast as possible. I'm strongly against run-time checks in critical path,
>>>>>> since they kill performance and make the whole approach a lot less usable.
>>>>>
>>>>> I may have described my suggestion poorly. I'm suggesting that all of
>>>>> these global ids be replaced *for userspace's benefit* with fds. That
>>>>> is, a map would have an associated struct inode, and, when you load an
>>>>> eBPF program, you'd pass fds into the kernel instead of global ids.
>>>>> The kernel would still compile the eBPF program to use the global ids,
>>>>> though.
>>>>
>>>> Hmm. If I understood you correctly, you're suggesting to do it similar
>>>> to ipc/mqueue, shmem, sockets do. By registering and mounting
>>>> a file system and providing all superblock and inode hooks… and
>>>> probably have its own namespace type… hmm… may be. That's
>>>> quite a bit of work to put lightly. As I said in the other email the first
>>>> step is root only and all these complexity just not worth doing
>>>> at this stage.
>>>
>>> The downside of not doing it right away is that it's harder to
>>> retrofit in without breaking early users.
>>>
>>> You might be able to get away with using anon_inodes. That will
>>
>> Spent quite a bit of time playing with anon_inode_getfd(). The model
>> works ok for seccomp, but doesn't seem to work for tracing,
>> since tracepoints are global. Say, syscall(bpf, load_prog) returns
>> a process-local fd. This 'fd' as a string can be written to
>> debugfs/tracing/events/.../filter which will increment a refcnt of a global
>> ebpf_program structure and will keep using it. When process exits it will
>> close all fds which in case of ebpf_prog_fd should be a nop, since
>> the program is still attached to a global event. Now we have a
>> program and maps that still alive and dangling, since tracepoint events
>> keep coming, but no new process can access it. Here we just lost all
>> benefits of making it 'fd' based. Theoretically we can extend tracing to
>> be fd-based too and tracepoints will auto-detach upon process exit,
>> but that's not going to work for all other global events. Like networking
>> components (bridge, ovs, …) are global and they won't be adding
>> fd-based interfaces.
>> I'm still thinking about it, but it looks like that any process-local
>> ebpf_prog_id scheme is not going to work for global events. Thoughts?
>
> Hmm. Maybe these things do need global ids for tracing, or at least
> there need to be some way to stash them somewhere and find them again.
> I suppose that debugfs could have symlinks to them, but I don't know
> how hard that would be to implement or how awkward it would be to use.
>
> I imagine there's some awkwardness regardless. For tracing, if I
> create map 75 and eBPF program 492 that uses map 75, then I still need
> to remember that map 75 is the map I want (or I need to parse the eBPF
> program later on).
>
> How do you imagine the userspace code working? Maybe it would make
> sense to add some nlattrs for eBPF programs to map between referenced
> objects and nicknames for them. Then user code could look at
> /sys/kernel/debug/whatever/nickname_of_map to resolve the map id or
> even just open it directly.
I want to avoid string names, since they will force new 'strtab', 'symtab'
sections in the programs/maps and will uglify the user interface quite a bit.
Back in september one loadable unit was: one eBPF program + set of maps,
but tracing requirements forced a change, since multiple programs need
to access the same map and maps may need to be pre-populated before
the programs start executing, so I've split maps and programs into mostly
independent entities, but programs still need to think of maps as local:
For example I want to do a skb leak check 'tracing filter':
- attach this program to kretprobe of __alloc_skb():
u64 key = (u64) skb;
u64 value = bpf_get_time();
bpf_update_map_elem(1/*const_map_id*/, &key, &value);
- attach this program to consume_skb and kfree_skb tracepoints:
u64 key = (u64) skb;
bpf_delete_map_elem(1/*const_map_id*/, &key);
- and have user space do:
prior to loading:
bpf_create_map(1/*map_id*/, 8/*key_size*/, 8/*value*/, 1M /*max_entries*/)
and then periodically iterate the map to see whether any skb stayed
in the map for too long.
Programs need to be written with hard coded map_ids otherwise usability
suffers, so I did global 32-bit id in this RFC, but this indeed doesn't work
for unprivileged chrome browser unless programs are previously loaded
by root and chrome only does attach to seccomp.
So here is the non-root bpf syscall interface I'm thinking about:
ufd = bpf_create_map(map_id, key_size, value_size, max_entries);
it will create a global map in the system which will be accessible
in this process via 'ufd'. Internally this 'ufd' will be assigned global map_id
and process-local map_id that was passed as a 1st argument.
To do update/lookup the process will use bpf_map_xxx_elem(ufd,…)
Then to load eBPF program the process will do:
ufd = bpf_prog_load(prog_type, ebpf_insn_array, license)
and instructions will be referring to maps via local map_id that
was hard coded as part of the program.
Beyond the normal create_map, update/lookup/delete, load_prog
operations (that are accessible to both root and non-root), the root user
gains one more operations: bpf_get_global_id(ufd) that returns
global map_id or prog_id. This id can be attached to global events
like tracing. Non-root users lose ability to do delete_map and
unload_prog (they do close(ufd) instead), so this ops are for root
only and operate on global ids.
This is the cleanest way I could think of to combine non-root
security, per-process id and global id all in one API. Thoughts?
^ permalink raw reply
* Re: [PATCH RFC net-next 01/14] net: filter: split filter.c into two files
From: Alexei Starovoitov @ 2014-07-02 5:35 UTC (permalink / raw)
To: Namhyung Kim
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
Network Development, LKML
In-Reply-To: <8738ek5qyh.fsf-vfBCOVm4yAnB69T4xOojN9BPR1lH4CV8@public.gmane.org>
On Tue, Jul 1, 2014 at 9:23 PM, Namhyung Kim <namhyung-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Alexei,
>
> On Fri, 27 Jun 2014 17:05:53 -0700, Alexei Starovoitov wrote:
>> BPF is used in several kernel components. This split creates logical boundary
>> between generic eBPF core and the rest
>>
>> kernel/bpf/core.c: eBPF interpreter
>>
>> net/core/filter.c: classic->eBPF converter, classic verifiers, socket filters
>>
>> This patch only moves functions.
>>
>> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
>> ---
>> kernel/Makefile | 1 +
>> kernel/bpf/Makefile | 1 +
>> kernel/bpf/core.c | 545 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> net/core/filter.c | 520 ------------------------------------------------
>> 4 files changed, 547 insertions(+), 520 deletions(-)
>> create mode 100644 kernel/bpf/Makefile
>> create mode 100644 kernel/bpf/core.c
>>
>> diff --git a/kernel/Makefile b/kernel/Makefile
>> index f2a8b6246ce9..e7360b7c2c0e 100644
>> --- a/kernel/Makefile
>> +++ b/kernel/Makefile
>> @@ -87,6 +87,7 @@ obj-$(CONFIG_RING_BUFFER) += trace/
>> obj-$(CONFIG_TRACEPOINTS) += trace/
>> obj-$(CONFIG_IRQ_WORK) += irq_work.o
>> obj-$(CONFIG_CPU_PM) += cpu_pm.o
>> +obj-$(CONFIG_NET) += bpf/
>
> But this still requires CONFIG_NET to use bpf. Why not adding
> CONFIG_BPF and making CONFIG_NET selects it?
This is the first patch that does 'split only'. Later patch replaces this line
with CONFIG_BPF.
^ permalink raw reply
* Re: [PATCH RFC net-next 08/14] bpf: add eBPF verifier
From: Alexei Starovoitov @ 2014-07-02 5:57 UTC (permalink / raw)
To: Namhyung Kim
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
Network Development, LKML
In-Reply-To: <87y4wc4aff.fsf@sejong.aot.lge.com>
On Tue, Jul 1, 2014 at 10:05 PM, Namhyung Kim <namhyung@gmail.com> wrote:
> Mostly questions and few nitpicks.. :)
great questions. Thank you for review! Answers below:
> On Fri, 27 Jun 2014 17:06:00 -0700, Alexei Starovoitov wrote:
>> +/* types of values:
>> + * - stored in an eBPF register
>> + * - passed into helper functions as an argument
>> + * - returned from helper functions
>> + */
>> +enum bpf_reg_type {
>> + INVALID_PTR, /* reg doesn't contain a valid pointer */
>
> I don't think it's a good name. The INVALID_PTR can be read as it
> contains a "pointer" which is invalid. Maybe INTEGER, NUMBER or
> something different can be used. And I think the struct reg_state->ptr
> should be renamed also.
ok. I agree that 'invalid' part of the name is too negative.
May be 'unknown_value' ?
>> + PTR_TO_CTX, /* reg points to bpf_context */
>> + PTR_TO_MAP, /* reg points to map element value */
>> + PTR_TO_MAP_CONDITIONAL, /* points to map element value or NULL */
>> + PTR_TO_STACK, /* reg == frame_pointer */
>> + PTR_TO_STACK_IMM, /* reg == frame_pointer + imm */
>> + PTR_TO_STACK_IMM_MAP_KEY, /* pointer to stack used as map key */
>> + PTR_TO_STACK_IMM_MAP_VALUE, /* pointer to stack used as map elem */
>
> So, these PTR_TO_STACK_IMM[_*] types are only for function argument,
> right? I guessed it could be used to access memory in general too, but
> then I thought it'd make verification complicated..
>
> And I also agree that it'd better splitting reg types and function
> argument constraints.
Ok. Will split this enum into three.
>> +
>> +/* check read/write into map element returned by bpf_table_lookup() */
>> +static int check_table_access(struct verifier_env *env, int regno, int off,
>> + int size)
>
> I guess the "table" is an old name of the "map"?
oops :) Yes. I've been calling them 'bpf tables' initially, but it created too
strong of a correlation to 'hash table', so I've changed the name to 'map'
to stress that this is a generic key/value and not just hash table.
>> + } else if (state->regs[regno].ptr == PTR_TO_STACK) {
>> + if (off >= 0 || off < -MAX_BPF_STACK) {
>> + verbose("invalid stack off=%d size=%d\n", off, size);
>> + return -EACCES;
>> + }
>
> So memory (stack) access is only allowed for a stack base regsiter and a
> constant offset, right?
Correct.
In other words it allows instructions:
BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_xx, -stack_offset);
Verifier makes no attempt to track pointer arithmetic and just marks
the result as 'invalid_ptr'.
For non-root programs it will reject programs that are trying to do
arithmetic on pointers (it's not part of this patch yet).
>> + /* check args */
>> + _(check_func_arg(env, BPF_REG_1, fn->arg1_type, &map_id, &map));
>> + _(check_func_arg(env, BPF_REG_2, fn->arg2_type, &map_id, &map));
>> + _(check_func_arg(env, BPF_REG_3, fn->arg3_type, &map_id, &map));
>> + _(check_func_arg(env, BPF_REG_4, fn->arg4_type, &map_id, &map));
>
> Missing BPF_REG_5?
yes. good catch.
I guess this shows that we didn't have a use case for function with 5 args :)
Will fix this.
>> +#define PEAK_INT() \
>
> s/PEAK/PEEK/ ?
aren't these the same? ;))
Will fix. Thanks!
^ permalink raw reply
* Re: [PATCH RFC net-next 11/14] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-07-02 6:14 UTC (permalink / raw)
To: Namhyung Kim
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
Arnaldo Carvalho de Melo, Jiri Olsa, Thomas Gleixner,
H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
Network Development, LKML
In-Reply-To: <87tx70496q.fsf-vfBCOVm4yAnB69T4xOojN9BPR1lH4CV8@public.gmane.org>
On Tue, Jul 1, 2014 at 10:32 PM, Namhyung Kim <namhyung-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 27 Jun 2014 17:06:03 -0700, Alexei Starovoitov wrote:
>> User interface:
>> cat bpf_123 > /sys/kernel/debug/tracing/__event__/filter
>>
>> where 123 is an id of the eBPF program priorly loaded.
>> __event__ is static tracepoint event.
>> (kprobe events will be supported in the future patches)
>>
>> eBPF programs can call in-kernel helper functions to:
>> - lookup/update/delete elements in maps
>> - memcmp
>> - trace_printk
>
> ISTR Steve doesn't like to use trace_printk() (at least for production
> kernels) anymore. And I'm not sure it'd work if there's no existing
> trace_printk() on a system.
yes. I saw big warning that trace_printk_init_buffers() emits.
The idea here is to use eBPF programs for live kernel debugging.
Instead of adding printk() and recompiling, just write a program,
attach it to some event, and printk whatever is interesting.
My only concern about printk() was that it dumps things into trace
buffers (which is still better than dumping stuff to syslog), but now
(since Andy almost convinced me to switch to 'fd' based interface)
we can have seq_printk-like that prints into special buffer. So that
user space does 'read(ufd)' and receives whatever program has
printed. I think that would be much cleaner.
>> + if (unlikely(ftrace_file->flags & FTRACE_EVENT_FL_FILTERED) && \
>> + unlikely(ftrace_file->event_call->flags & TRACE_EVENT_FL_BPF)) { \
>> + struct bpf_context __ctx; \
>> + \
>> + populate_bpf_context(&__ctx, args, 0, 0, 0, 0, 0); \
>> + trace_filter_call_bpf(ftrace_file->filter, &__ctx); \
>> + return; \
>> + } \
>> + \
>
> Hmm.. But it seems the eBPF prog is not a filter - it'd always drop the
> event. And I think it's better to use a recorded entry rather then args
> as a bpf_context so that tools like perf can manipulate it at compile
> time based on the event format.
Can manipulate what at compile time? Entry records of tracepoints are
hard coded based on the event. For verifier it's easier to treat all
tracepoint events as they received the same 'struct bpf_context'
of N arguments then the same program can be attached to multiple
tracepoint events at the same time.
I thought about making verifier specific for _every_ tracepoint event,
but it complicates the user interface, since 'bpf_context' is now different
for every program. I think args are much easier to deal with from C
programming point of view, since program can go a fetch the same
fields that tracepoint 'fast_assign' macro does.
Also skipping buffer allocation and fast_assign gives very sizable
performance boost, since the program will access only what it needs to.
The return value of eBPF program is ignored, since I couldn't think
of use case for it. We can change it to be more 'filter' like and interpret
return value as true/false, whether to record this event or not. Thoughts?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox