* [PATCH net-next v4 4/4] bpf: add a test case for helper bpf_perf_prog_read_value
From: Yonghong Song @ 2017-09-19 7:04 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170919070413.3838201-1-yhs@fb.com>
The bpf sample program trace_event is enhanced to use the new
helper to print out enabled/running time.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/trace_event_kern.c | 10 ++++++++++
samples/bpf/trace_event_user.c | 13 ++++++++-----
tools/include/uapi/linux/bpf.h | 3 ++-
tools/testing/selftests/bpf/bpf_helpers.h | 3 +++
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/samples/bpf/trace_event_kern.c b/samples/bpf/trace_event_kern.c
index 41b6115..a77a583d 100644
--- a/samples/bpf/trace_event_kern.c
+++ b/samples/bpf/trace_event_kern.c
@@ -37,10 +37,14 @@ struct bpf_map_def SEC("maps") stackmap = {
SEC("perf_event")
int bpf_prog1(struct bpf_perf_event_data *ctx)
{
+ char time_fmt1[] = "Time Enabled: %llu, Time Running: %llu";
+ char time_fmt2[] = "Get Time Failed, ErrCode: %d";
char fmt[] = "CPU-%d period %lld ip %llx";
u32 cpu = bpf_get_smp_processor_id();
+ struct bpf_perf_event_value value_buf;
struct key_t key;
u64 *val, one = 1;
+ int ret;
if (ctx->sample_period < 10000)
/* ignore warmup */
@@ -54,6 +58,12 @@ int bpf_prog1(struct bpf_perf_event_data *ctx)
return 0;
}
+ ret = bpf_perf_prog_read_value(ctx, (void *)&value_buf, sizeof(struct bpf_perf_event_value));
+ if (!ret)
+ bpf_trace_printk(time_fmt1, sizeof(time_fmt1), value_buf.enabled, value_buf.running);
+ else
+ bpf_trace_printk(time_fmt2, sizeof(time_fmt2), ret);
+
val = bpf_map_lookup_elem(&counts, &key);
if (val)
(*val)++;
diff --git a/samples/bpf/trace_event_user.c b/samples/bpf/trace_event_user.c
index 7bd827b..bf4f1b6 100644
--- a/samples/bpf/trace_event_user.c
+++ b/samples/bpf/trace_event_user.c
@@ -127,6 +127,9 @@ static void test_perf_event_all_cpu(struct perf_event_attr *attr)
int *pmu_fd = malloc(nr_cpus * sizeof(int));
int i, error = 0;
+ /* system wide perf event, no need to inherit */
+ attr->inherit = 0;
+
/* open perf_event on all cpus */
for (i = 0; i < nr_cpus; i++) {
pmu_fd[i] = sys_perf_event_open(attr, -1, i, -1, 0);
@@ -154,6 +157,11 @@ static void test_perf_event_task(struct perf_event_attr *attr)
{
int pmu_fd;
+ /* per task perf event, enable inherit so the "dd ..." command can be traced properly.
+ * Enabling inherit will cause bpf_perf_prog_read_time helper failure.
+ */
+ attr->inherit = 1;
+
/* open task bound event */
pmu_fd = sys_perf_event_open(attr, 0, -1, -1, 0);
if (pmu_fd < 0) {
@@ -175,14 +183,12 @@ static void test_bpf_perf_event(void)
.freq = 1,
.type = PERF_TYPE_HARDWARE,
.config = PERF_COUNT_HW_CPU_CYCLES,
- .inherit = 1,
};
struct perf_event_attr attr_type_sw = {
.sample_freq = SAMPLE_FREQ,
.freq = 1,
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_CPU_CLOCK,
- .inherit = 1,
};
struct perf_event_attr attr_hw_cache_l1d = {
.sample_freq = SAMPLE_FREQ,
@@ -192,7 +198,6 @@ static void test_bpf_perf_event(void)
PERF_COUNT_HW_CACHE_L1D |
(PERF_COUNT_HW_CACHE_OP_READ << 8) |
(PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16),
- .inherit = 1,
};
struct perf_event_attr attr_hw_cache_branch_miss = {
.sample_freq = SAMPLE_FREQ,
@@ -202,7 +207,6 @@ static void test_bpf_perf_event(void)
PERF_COUNT_HW_CACHE_BPU |
(PERF_COUNT_HW_CACHE_OP_READ << 8) |
(PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
- .inherit = 1,
};
struct perf_event_attr attr_type_raw = {
.sample_freq = SAMPLE_FREQ,
@@ -210,7 +214,6 @@ static void test_bpf_perf_event(void)
.type = PERF_TYPE_RAW,
/* Intel Instruction Retired */
.config = 0xc0,
- .inherit = 1,
};
printf("Test HW_CPU_CYCLES\n");
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 79eb529..fa1be2c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -633,7 +633,8 @@ union bpf_attr {
FN(redirect_map), \
FN(sk_redirect_map), \
FN(sock_map_update), \
- FN(perf_event_read_value),
+ FN(perf_event_read_value), \
+ FN(perf_prog_read_value),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index c866682..892d785 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -73,6 +73,9 @@ static int (*bpf_sock_map_update)(void *map, void *key, void *value,
static int (*bpf_perf_event_read_value)(void *map, unsigned long long flags,
void *buf, unsigned int buf_size) =
(void *) BPF_FUNC_perf_event_read_value;
+static int (*bpf_perf_prog_read_value)(void *ctx, void *buf,
+ unsigned int buf_size) =
+ (void *) BPF_FUNC_perf_prog_read_value;
/* llvm builtin functions that eBPF C program may use to
--
2.9.5
^ permalink raw reply related
* [PATCH net-next v4 1/4] bpf: add helper bpf_perf_event_read_value for perf event array map
From: Yonghong Song @ 2017-09-19 7:04 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170919070413.3838201-1-yhs@fb.com>
Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.
Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
normalized_num_samples = num_samples * time_enabled / time_running
normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.
This patch adds helper bpf_perf_event_read_value for kprobed based perf
event array map, to read perf counter and enabled/running time.
The enabled/running time is accumulated since the perf event open.
To achieve scaling factor between two bpf invocations, users
can can use cpu_id as the key (which is typical for perf array usage model)
to remember the previous value and do the calculation inside the
bpf program.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/perf_event.h | 6 ++++--
include/uapi/linux/bpf.h | 18 +++++++++++++++++-
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/verifier.c | 4 +++-
kernel/events/core.c | 15 ++++++++++++---
kernel/trace/bpf_trace.c | 44 ++++++++++++++++++++++++++++++++++++++++----
6 files changed, 77 insertions(+), 12 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 8e22f24..21d8c12 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -884,7 +884,8 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr,
void *context);
extern void perf_pmu_migrate_context(struct pmu *pmu,
int src_cpu, int dst_cpu);
-int perf_event_read_local(struct perf_event *event, u64 *value);
+int perf_event_read_local(struct perf_event *event, u64 *value,
+ u64 *enabled, u64 *running);
extern u64 perf_event_read_value(struct perf_event *event,
u64 *enabled, u64 *running);
@@ -1286,7 +1287,8 @@ static inline const struct perf_event_attr *perf_event_attrs(struct perf_event *
{
return ERR_PTR(-EINVAL);
}
-static inline int perf_event_read_local(struct perf_event *event, u64 *value)
+static inline int perf_event_read_local(struct perf_event *event, u64 *value,
+ u64 *enabled, u64 *running)
{
return -EINVAL;
}
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 43ab5c4..2c68b9e 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -582,6 +582,14 @@ union bpf_attr {
* @map: pointer to sockmap to update
* @key: key to insert/update sock in map
* @flags: same flags as map update elem
+ *
+ * int bpf_perf_event_read_value(map, flags, buf, buf_size)
+ * read perf event counter value and perf event enabled/running time
+ * @map: pointer to perf_event_array map
+ * @flags: index of event in the map or bitmask flags
+ * @buf: buf to fill
+ * @buf_size: size of the buf
+ * Return: 0 on success or negative error code
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -638,6 +646,7 @@ union bpf_attr {
FN(redirect_map), \
FN(sk_redirect_map), \
FN(sock_map_update), \
+ FN(perf_event_read_value), \
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -681,7 +690,8 @@ enum bpf_func_id {
#define BPF_F_ZERO_CSUM_TX (1ULL << 1)
#define BPF_F_DONT_FRAGMENT (1ULL << 2)
-/* BPF_FUNC_perf_event_output and BPF_FUNC_perf_event_read flags. */
+/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
+ * BPF_FUNC_perf_event_read_value flags. */
#define BPF_F_INDEX_MASK 0xffffffffULL
#define BPF_F_CURRENT_CPU BPF_F_INDEX_MASK
/* BPF_FUNC_perf_event_output for sk_buff input context. */
@@ -864,4 +874,10 @@ enum {
#define TCP_BPF_IW 1001 /* Set TCP initial congestion window */
#define TCP_BPF_SNDCWND_CLAMP 1002 /* Set sndcwnd_clamp */
+struct bpf_perf_event_value {
+ __u64 counter;
+ __u64 enabled;
+ __u64 running;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 98c0f00..68d8666 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -492,7 +492,7 @@ static void *perf_event_fd_array_get_ptr(struct bpf_map *map,
ee = ERR_PTR(-EOPNOTSUPP);
event = perf_file->private_data;
- if (perf_event_read_local(event, &value) == -EOPNOTSUPP)
+ if (perf_event_read_local(event, &value, NULL, NULL) == -EOPNOTSUPP)
goto err_out;
ee = bpf_event_entry_gen(perf_file, map_file);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 799b245..1bf9d7b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1494,7 +1494,8 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
break;
case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
if (func_id != BPF_FUNC_perf_event_read &&
- func_id != BPF_FUNC_perf_event_output)
+ func_id != BPF_FUNC_perf_event_output &&
+ func_id != BPF_FUNC_perf_event_read_value)
goto error;
break;
case BPF_MAP_TYPE_STACK_TRACE:
@@ -1537,6 +1538,7 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
break;
case BPF_FUNC_perf_event_read:
case BPF_FUNC_perf_event_output:
+ case BPF_FUNC_perf_event_read_value:
if (map->map_type != BPF_MAP_TYPE_PERF_EVENT_ARRAY)
goto error;
break;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3e691b7..2d5bbe5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3684,10 +3684,12 @@ static inline u64 perf_event_count(struct perf_event *event)
* will not be local and we cannot read them atomically
* - must not have a pmu::count method
*/
-int perf_event_read_local(struct perf_event *event, u64 *value)
+int perf_event_read_local(struct perf_event *event, u64 *value,
+ u64 *enabled, u64 *running)
{
unsigned long flags;
int ret = 0;
+ u64 now;
/*
* Disabling interrupts avoids all counter scheduling (context
@@ -3718,14 +3720,21 @@ int perf_event_read_local(struct perf_event *event, u64 *value)
goto out;
}
+ now = event->shadow_ctx_time + perf_clock();
+ if (enabled)
+ *enabled = now - event->tstamp_enabled;
/*
* If the event is currently on this CPU, its either a per-task event,
* or local to this CPU. Furthermore it means its ACTIVE (otherwise
* oncpu == -1).
*/
- if (event->oncpu == smp_processor_id())
+ if (event->oncpu == smp_processor_id()) {
event->pmu->read(event);
-
+ if (running)
+ *running = now - event->tstamp_running;
+ } else if (running) {
+ *running = event->total_time_running;
+ }
*value = local64_read(&event->count);
out:
local_irq_restore(flags);
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index dc498b6..39ce5d9 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -255,13 +255,13 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
return &bpf_trace_printk_proto;
}
-BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
-{
+static __always_inline int
+get_map_perf_counter(struct bpf_map *map, u64 flags,
+ u64 *value, u64 *enabled, u64 *running) {
struct bpf_array *array = container_of(map, struct bpf_array, map);
unsigned int cpu = smp_processor_id();
u64 index = flags & BPF_F_INDEX_MASK;
struct bpf_event_entry *ee;
- u64 value = 0;
int err;
if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
@@ -275,7 +275,17 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
if (!ee)
return -ENOENT;
- err = perf_event_read_local(ee->event, &value);
+ err = perf_event_read_local(ee->event, value, enabled, running);
+ return err;
+}
+
+
+BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
+{
+ u64 value = 0;
+ int err;
+
+ err = get_map_perf_counter(map, flags, &value, NULL, NULL);
/*
* this api is ugly since we miss [-22..-2] range of valid
* counter values, but that's uapi
@@ -285,6 +295,20 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
return value;
}
+BPF_CALL_4(bpf_perf_event_read_value, struct bpf_map *, map, u64, flags,
+ struct bpf_perf_event_value *, buf, u32, size)
+{
+ int err;
+
+ if (unlikely(size != sizeof(struct bpf_perf_event_value)))
+ return -EINVAL;
+ err = get_map_perf_counter(map, flags, &buf->counter, &buf->enabled,
+ &buf->running);
+ if (err)
+ return err;
+ return 0;
+}
+
static const struct bpf_func_proto bpf_perf_event_read_proto = {
.func = bpf_perf_event_read,
.gpl_only = true,
@@ -293,6 +317,16 @@ static const struct bpf_func_proto bpf_perf_event_read_proto = {
.arg2_type = ARG_ANYTHING,
};
+static const struct bpf_func_proto bpf_perf_event_read_value_proto = {
+ .func = bpf_perf_event_read_value,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg4_type = ARG_CONST_SIZE,
+};
+
static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
static __always_inline u64
@@ -499,6 +533,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
return &bpf_perf_event_output_proto;
case BPF_FUNC_get_stackid:
return &bpf_get_stackid_proto;
+ case BPF_FUNC_perf_event_read_value:
+ return &bpf_perf_event_read_value_proto;
default:
return tracing_func_proto(func_id);
}
--
2.9.5
^ permalink raw reply related
* Re: [PATCH net-next v3 1/4] bpf: add helper bpf_perf_event_read_value for perf event array map
From: kbuild test robot @ 2017-09-19 6:28 UTC (permalink / raw)
To: Yonghong Song
Cc: kbuild-all, peterz, rostedt, ast, daniel, netdev, kernel-team
In-Reply-To: <20170918230553.1624357-2-yhs@fb.com>
[-- Attachment #1: Type: text/plain, Size: 1991 bytes --]
Hi Yonghong,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Yonghong-Song/bpf-add-two-helpers-to-read-perf-event-enabled-running-time/20170919-134113
config: m68k-allyesconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k
All errors (new ones prefixed by >>):
kernel/bpf/arraymap.c: In function 'perf_event_fd_array_get_ptr':
>> kernel/bpf/arraymap.c:495:6: error: too many arguments to function 'perf_event_read_local'
if (perf_event_read_local(event, &value, NULL, NULL) == -EOPNOTSUPP)
^
In file included from kernel/bpf/arraymap.c:18:0:
include/linux/perf_event.h:1290:19: note: declared here
static inline int perf_event_read_local(struct perf_event *event, u64 *value)
^
vim +/perf_event_read_local +495 kernel/bpf/arraymap.c
480
481 static void *perf_event_fd_array_get_ptr(struct bpf_map *map,
482 struct file *map_file, int fd)
483 {
484 struct bpf_event_entry *ee;
485 struct perf_event *event;
486 struct file *perf_file;
487 u64 value;
488
489 perf_file = perf_event_get(fd);
490 if (IS_ERR(perf_file))
491 return perf_file;
492
493 ee = ERR_PTR(-EOPNOTSUPP);
494 event = perf_file->private_data;
> 495 if (perf_event_read_local(event, &value, NULL, NULL) == -EOPNOTSUPP)
496 goto err_out;
497
498 ee = bpf_event_entry_gen(perf_file, map_file);
499 if (ee)
500 return ee;
501 ee = ERR_PTR(-ENOMEM);
502 err_out:
503 fput(perf_file);
504 return ee;
505 }
506
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44401 bytes --]
^ permalink raw reply
* Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Corentin Labbe @ 2017-09-19 5:31 UTC (permalink / raw)
To: robh-DgEjT+Ai2ygdnm+yROfE0A
Cc: Rob Herring, Maxime Ripard, mark.rutland-5wv7dgnIgG8,
wens-jdAy2FN1RRM, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
peppe.cavallaro-qxv4g6HH51o, alexandre.torgue-qxv4g6HH51o,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, andrew-g2DYL2Zd6BY
In-Reply-To: <20170914191949.GA3796-g2DYL2Zd6BY@public.gmane.org>
On Thu, Sep 14, 2017 at 09:19:49PM +0200, Andrew Lunn wrote:
> > > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"?
> > > If the latter, then I think the node is fine, but then the mux should be
> > > a child node of it. IOW, the child of an MDIO controller should either
> > > be a mux node or slave devices.
>
> Hi Rob
>
> Up until now, children of an MDIO bus have been MDIO devices. Those
> MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
> oddball devices that Broadcom iProc has, like generic PHYs.
>
> We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
> device, and does not have the properties of an MDIO device. It is not
> addressable on the MDIO bus. The current MUXes are addressed via GPIOs
> or MMIO.
>
> There other similar cases. i2c-mux-gpio is not a child of an i2c bus,
> nor i2c-mux-reg or gpio-mux. nxp,pca9548 is however a child of the i2c
> bus, because it is an i2c device itself...
>
> If the MDIO mux was an MDIO device, i would agree with you. Bit it is
> not, so lets not make it a child.
>
> Andrew
Hello Rob, could you anwser/confirm please.
I wait on this for sending the next version.
Thanks
Regards
Corentin Labbe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-19 5:22 UTC (permalink / raw)
To: Vinicius Costa Gomes
Cc: netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan, andre.guedes,
ivan.briano, jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <87k20vip3f.fsf@intel.com>
On Mon, Sep 18, 2017 at 04:06:28PM -0700, Vinicius Costa Gomes wrote:
> That's the point, the application does not need to know that, and asking
> that would be stupid.
On the contrary, this information is essential to the application.
Probably you have never seen an actual Ethernet field bus in
operation? In any case, you are missing the point.
> (And that's another nice point of how 802.1Qbv works, applications do
> not need to be changed to use it, and I think we should work to achieve
> this on the Linux side)
Once you start to care about real time performance, then you need to
consider the applications. This is industrial control, not streaming
your tunes from your ipod.
> That being said, that only works for kinds of traffic that maps well to
> this configuration in advance model, which is the model that the IEEE
> (see 802.1Qcc) and the AVNU Alliance[1] are pushing for.
Again, you are missing the point of what they aiming for. I have
looked at a number of production systems, and in each case the
developers want total control over the transmission, in order to
reduce latency to an absolute minimum. Typically the data to be sent
are available only microseconds before the transmission deadline.
Consider OpenAVB on github that people are already using. Take a look
at simple_talker.c and explain how "applications do not need to be
changed to use it."
> [1]
> http://avnu.org/theory-of-operation-for-tsn-enabled-industrial-systems/
Did you even read this?
[page 24]
As described in section 2, some industrial control systems require
predictable, very low latency and cycle-to-cycle variation to meet
hard real-time application requirements. In these systems,
multiple distributed controllers commonly synchronize their
sensor/actuator operations with other controllers by scheduling
these operations in time, typically using a repeating control
cycle.
...
The gate control mechanism is itself a time-aware PTP application
operating within a bridge or end station port.
It is an application, not a "god box."
> In short, I see a per-packet transmission time and a per-queue schedule
> as solutions to different problems.
Well, I can agree with that. For some non real-time applications,
bandwidth shaping is enough, and your Qdisc idea is sufficient. For
the really challenging TSN targets (industrial control, automotive),
your idea of an opaque schedule file won't fly.
Thanks,
Richard
^ permalink raw reply
* [PATCH net-next] net: sk_buff rbnode reorg
From: Eric Dumazet @ 2017-09-19 5:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
skb->rbnode shares space with skb->next, skb->prev and skb->tstamp
Current uses (TCP receive ofo queue and netem) need to save/restore
tstamp.
Since we might use an RB tree for TCP retransmit queue at some point
to speedup SACK processing with large rtx queues, this patch exchanges
skb->dev and skb->tstamp.
This saves some overhead in both TCP and netem.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/skbuff.h | 16 ++++++++--------
include/net/tcp.h | 5 -----
net/ipv4/tcp_input.c | 27 +++++----------------------
net/sched/sch_netem.c | 7 ++++---
4 files changed, 17 insertions(+), 38 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 72299ef00061db1ce70d34b96ae1639ecde08837..492828801acba42ac6bccb287d3cc5080039135c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -661,8 +661,12 @@ struct sk_buff {
struct sk_buff *prev;
union {
- ktime_t tstamp;
- u64 skb_mstamp;
+ struct net_device *dev;
+ /* Some protocols might use this space to store information,
+ * while device pointer would be NULL.
+ * UDP receive path is one user.
+ */
+ unsigned long dev_scratch;
};
};
struct rb_node rbnode; /* used in netem & tcp stack */
@@ -670,12 +674,8 @@ struct sk_buff {
struct sock *sk;
union {
- struct net_device *dev;
- /* Some protocols might use this space to store information,
- * while device pointer would be NULL.
- * UDP receive path is one user.
- */
- unsigned long dev_scratch;
+ ktime_t tstamp;
+ u64 skb_mstamp;
};
/*
* This is the control buffer. It is free to use for every
diff --git a/include/net/tcp.h b/include/net/tcp.h
index b510f284427aabc1f508d24d29d0f812e5e0aa61..6ecc01aa667b26a3e2270a3566b2076bfebd8605 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -797,11 +797,6 @@ struct tcp_skb_cb {
u16 tcp_gso_segs;
u16 tcp_gso_size;
};
-
- /* Used to stash the receive timestamp while this skb is in the
- * out of order queue, as skb->tstamp is overwritten by the
- * rbnode.
- */
ktime_t swtstamp;
};
__u8 tcp_flags; /* TCP header flags. (tcp[13]) */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bddf724f5c02abe1d11110cffe2517f6376e440d..db9bb46b5776f9ee332298c0e95afb0a5966b938 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4266,11 +4266,6 @@ static void tcp_sack_remove(struct tcp_sock *tp)
tp->rx_opt.num_sacks = num_sacks;
}
-enum tcp_queue {
- OOO_QUEUE,
- RCV_QUEUE,
-};
-
/**
* tcp_try_coalesce - try to merge skb to prior one
* @sk: socket
@@ -4286,7 +4281,6 @@ enum tcp_queue {
* Returns true if caller should free @from instead of queueing it
*/
static bool tcp_try_coalesce(struct sock *sk,
- enum tcp_queue dest,
struct sk_buff *to,
struct sk_buff *from,
bool *fragstolen)
@@ -4311,10 +4305,7 @@ static bool tcp_try_coalesce(struct sock *sk,
if (TCP_SKB_CB(from)->has_rxtstamp) {
TCP_SKB_CB(to)->has_rxtstamp = true;
- if (dest == OOO_QUEUE)
- TCP_SKB_CB(to)->swtstamp = TCP_SKB_CB(from)->swtstamp;
- else
- to->tstamp = from->tstamp;
+ to->tstamp = from->tstamp;
}
return true;
@@ -4351,9 +4342,6 @@ static void tcp_ofo_queue(struct sock *sk)
}
p = rb_next(p);
rb_erase(&skb->rbnode, &tp->out_of_order_queue);
- /* Replace tstamp which was stomped by rbnode */
- if (TCP_SKB_CB(skb)->has_rxtstamp)
- skb->tstamp = TCP_SKB_CB(skb)->swtstamp;
if (unlikely(!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt))) {
SOCK_DEBUG(sk, "ofo packet was already received\n");
@@ -4365,8 +4353,7 @@ static void tcp_ofo_queue(struct sock *sk)
TCP_SKB_CB(skb)->end_seq);
tail = skb_peek_tail(&sk->sk_receive_queue);
- eaten = tail && tcp_try_coalesce(sk, RCV_QUEUE,
- tail, skb, &fragstolen);
+ eaten = tail && tcp_try_coalesce(sk, tail, skb, &fragstolen);
tcp_rcv_nxt_update(tp, TCP_SKB_CB(skb)->end_seq);
fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
if (!eaten)
@@ -4420,10 +4407,6 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
return;
}
- /* Stash tstamp to avoid being stomped on by rbnode */
- if (TCP_SKB_CB(skb)->has_rxtstamp)
- TCP_SKB_CB(skb)->swtstamp = skb->tstamp;
-
/* Disable header prediction. */
tp->pred_flags = 0;
inet_csk_schedule_ack(sk);
@@ -4451,7 +4434,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
/* In the typical case, we are adding an skb to the end of the list.
* Use of ooo_last_skb avoids the O(Log(N)) rbtree lookup.
*/
- if (tcp_try_coalesce(sk, OOO_QUEUE, tp->ooo_last_skb,
+ if (tcp_try_coalesce(sk, tp->ooo_last_skb,
skb, &fragstolen)) {
coalesce_done:
tcp_grow_window(sk, skb);
@@ -4502,7 +4485,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
__kfree_skb(skb1);
goto merge_right;
}
- } else if (tcp_try_coalesce(sk, OOO_QUEUE, skb1,
+ } else if (tcp_try_coalesce(sk, skb1,
skb, &fragstolen)) {
goto coalesce_done;
}
@@ -4554,7 +4537,7 @@ static int __must_check tcp_queue_rcv(struct sock *sk, struct sk_buff *skb, int
__skb_pull(skb, hdrlen);
eaten = (tail &&
- tcp_try_coalesce(sk, RCV_QUEUE, tail,
+ tcp_try_coalesce(sk, tail,
skb, fragstolen)) ? 1 : 0;
tcp_rcv_nxt_update(tcp_sk(sk), TCP_SKB_CB(skb)->end_seq);
if (!eaten) {
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index b1266e75ca43cf5a66b951ecabccfc5b24069444..063a4bdb9ee6f26b01387959e8f6ccd15ec16191 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -146,7 +146,6 @@ struct netem_sched_data {
*/
struct netem_skb_cb {
psched_time_t time_to_send;
- ktime_t tstamp_save;
};
@@ -561,7 +560,6 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
}
cb->time_to_send = now + delay;
- cb->tstamp_save = skb->tstamp;
++q->counter;
tfifo_enqueue(skb, sch);
} else {
@@ -629,7 +627,10 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
qdisc_qstats_backlog_dec(sch, skb);
skb->next = NULL;
skb->prev = NULL;
- skb->tstamp = netem_skb_cb(skb)->tstamp_save;
+ /* skb->dev shares skb->rbnode area,
+ * we need to restore its value.
+ */
+ skb->dev = qdisc_dev(sch);
#ifdef CONFIG_NET_CLS_ACT
/*
^ permalink raw reply related
* linux-next: Signed-off-by missing for commit in the net tree
From: Stephen Rothwell @ 2017-09-19 4:43 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Eric Dumazet,
Ville Syrjälä
Hi all,
Commit
129c6cda2de2 ("8139too: revisit napi_complete_done() usage")
is missing a Signed-off-by from its author.
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: [PATCH net-next 12/14] gtp: Configuration for zero UDP checksum
From: David Miller @ 2017-09-19 4:24 UTC (permalink / raw)
To: tom; +Cc: netdev, pablo, laforge, rohit
In-Reply-To: <20170919003904.5124-13-tom@quantonium.net>
From: Tom Herbert <tom@quantonium.net>
Date: Mon, 18 Sep 2017 17:39:02 -0700
> Add configuration to control use of zero checksums on transmit for both
> IPv4 and IPv6, and control over accepting zero IPv6 checksums on
> receive.
>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
I thought we were trying to move away from this special case of allowing
zero UDP checksums with tunnels, especially for ipv6.
^ permalink raw reply
* Re: [PATCH net-next 11/14] net: Add a facility to support application defined GSO
From: David Miller @ 2017-09-19 4:21 UTC (permalink / raw)
To: tom; +Cc: netdev, pablo, laforge, rohit
In-Reply-To: <20170919003904.5124-12-tom@quantonium.net>
From: Tom Herbert <tom@quantonium.net>
Date: Mon, 18 Sep 2017 17:39:01 -0700
> Allow applications or encapsulation protocols to register a GSO segment
> function to their specific protocol. To faciliate this I reserved the
> upper four bits in the gso_type to indicate the application specific GSO
> type. Zero in these bits indicates no application GSO, so there are
> fifteen instance that can be defined.
>
> An application registers a a gso_segment using the skb_gso_app_register
> this takes a struct skb_gso_app that indicates a callback function as
> well as a set of GSO types for which at least one must be matched before
> calling he segment function. GSO returns one of the application GSO
> types described above (not a fixed value for the applications).
> Subsequently, when the application sends a GSO packet the application
> gso_type is set in the skb gso_type along with any other types.
>
> skb_gso_app_segment is the function called from another GSO segment
> function to handle segmentation of the application or encapsulation
> protocol. This function includes check flags that provides context for
> the appropriate GSO instance to match. For instance, in order to handle
> a protocol encapsulated in UDP (GTP for instance) skb_gso_app_segment is
> call from udp_tunnel_segment and check flags would be
> SKB_GSO_UDP_TUNNEL_CSUM | SKB_GSO_UDP_TUNNEL.
>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
What happens on cards that can offload existing arbitrary UDP tunnel
encapsulations?
Will something about the state of the GSO type bits you are adding
prevent that? Or do we need to add some new checks somewhere?
^ permalink raw reply
* Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6
From: David Miller @ 2017-09-19 4:19 UTC (permalink / raw)
To: tom; +Cc: netdev, pablo, laforge, rohit
In-Reply-To: <20170919003904.5124-9-tom@quantonium.net>
From: Tom Herbert <tom@quantonium.net>
Date: Mon, 18 Sep 2017 17:38:58 -0700
> Allow peers to be specified by IPv6 addresses.
>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
Hmmm, can you just check the socket family or something like that?
^ permalink raw reply
* Re: [PATCH net-next 07/14] gtp: Support encapsulation of IPv6 packets
From: David Miller @ 2017-09-19 4:19 UTC (permalink / raw)
To: tom; +Cc: netdev, pablo, laforge, rohit
In-Reply-To: <20170919003904.5124-8-tom@quantonium.net>
From: Tom Herbert <tom@quantonium.net>
Date: Mon, 18 Sep 2017 17:38:57 -0700
> @@ -98,6 +104,7 @@ static void pdp_context_delete(struct pdp_ctx *pctx);
> static inline u32 gtp0_hashfn(u64 tid)
> {
> u32 *tid32 = (u32 *) &tid;
> +
> return jhash_2words(tid32[0], tid32[1], gtp_h_initval);
> }
>
> @@ -111,6 +118,11 @@ static inline u32 ipv4_hashfn(__be32 ip)
> return jhash_1word((__force u32)ip, gtp_h_initval);
> }
>
> +static inline u32 ipv6_hashfn(const struct in6_addr *a)
> +{
> + return __ipv6_addr_jhash(a, gtp_h_initval);
> +}
I know you are just following the pattern of the existing "ipv4_hashfn()" here
but this kind of stuff is not very global namespace friendly. Even simply
adding a "gtp_" prefix to these hash functions would be a lot better.
^ permalink raw reply
* Re: [PATCH net-next 03/14] gtp: Call common functions to get tunnel routes and add dst_cache
From: David Miller @ 2017-09-19 4:17 UTC (permalink / raw)
To: tom; +Cc: netdev, pablo, laforge, rohit
In-Reply-To: <20170919003904.5124-4-tom@quantonium.net>
From: Tom Herbert <tom@quantonium.net>
Date: Mon, 18 Sep 2017 17:38:53 -0700
> Call ip_tunnel_get_route and dst_cache to pdp context which should
> improve performance by obviating the need to perform a route lookup
> on every packet.
>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
Not caused by your changes, but something to think about:
> -static struct rtable *ip4_route_output_gtp(struct flowi4 *fl4,
> - const struct sock *sk,
> - __be32 daddr)
> -{
> - memset(fl4, 0, sizeof(*fl4));
> - fl4->flowi4_oif = sk->sk_bound_dev_if;
> - fl4->daddr = daddr;
> - fl4->saddr = inet_sk(sk)->inet_saddr;
> - fl4->flowi4_tos = RT_CONN_FLAGS(sk);
> - fl4->flowi4_proto = sk->sk_protocol;
> -
> - return ip_route_output_key(sock_net(sk), fl4);
> -}
This and the new dst caching code ignores any source address selection
done by ip_route_output_key() or the new tunnel route lookup helpers.
Either source address selection should be respected, or if saddr will
never be modified by a route lookup for some specific reason here,
that should be documented.
^ permalink raw reply
* Re: RFC: Audit Kernel Container IDs
From: Richard Guy Briggs @ 2017-09-19 4:15 UTC (permalink / raw)
To: Eric W. Biederman
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, Linux Containers, Linux API,
Linux Audit, Linux FS Devel, Linux Kernel,
Linux Network Development, Aristeu Rozanski, David Howells,
Eric Paris, jlayton-H+wXaHxf7aLQT0dZR+AlfA, Andy Lutomirski,
mszeredi-H+wXaHxf7aLQT0dZR+AlfA, Paul Moore, Serge E. Hallyn,
Steve Grubb, trondmy-7I+n7zu2hftEKMMhf/gKZA, Al Viro
In-Reply-To: <87wp4v76f4.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On 2017-09-18 21:45, Eric W. Biederman wrote:
> Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
>
> > On 2017-09-14 12:33, Eric W. Biederman wrote:
> >> Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
> >>
> >> > The trigger is a pseudo filesystem (proc, since PID tree already exists)
> >> > write of a u64 representing the container ID to a file representing a
> >> > process that will become the first process in a new container.
> >> > This might place restrictions on mount namespaces required to define a
> >> > container, or at least careful checking of namespaces in the kernel to
> >> > verify permissions of the orchestrator so it can't change its own
> >> > container ID.
> >>
> >> Why a u64?
> >
> > u32 will roll too quickly. UUID is large enough that it adds
> > significantly to audit record bandwidth. I'd prefer u64, but can look
> > at the difference of accommodating a UUID...
>
> I was imagining a string might be better. As for the purposes of audit
> it is just a byte string you regurgitate.
Yes, so looking at u128 vs dhowells' proposal, it would be 16 bytes vs
24 bytes, which really isn't that much difference...
What length of string length were you envisioning?
> >> Why a proc filesystem write and not a magic audit message?
> >
> > A magic audit message requires CAP_AUDIT_WRITE, which we'd like to use
> > sparingly. Given that orchestrators will already require it to send
> > the mandatory AUDIT_VIRT_*, this doesn't seem like an unreasonable burden.
> >
> > I was originally leaning towards an audit message trigger or a syscall.
> >
> >> I don't like the fact that the proc filesystem entry is likely going to
> >> be readable and abusable by non-audit contexts?
> >
> > This proposal wasn't going to start with that link being readable, but
> > its filesystem structure and link names would be, perhaps giving away
> > too much already.
> >
> > I think we will need to find a way for the orchestrator or one of its
> > authorized agents to read this information while blocking reads from
> > unauthorized agents, otherwise this would be of very limited use.
>
> Something that is set only for future audit messages seems reasonable.
> Once you start reading this from something other than audit messages I
> get neverous, that people will use this beyond audit for things it is
> not intended for.
Understandably. At the same time, if we implement something that is
more broadly useful and solves a number of other challenges others are
facing, how can we make it available while limiting the potential for
abuse?
> >> Why the ability to change the containerid? What is the use case you are
> >> thinking of there?
> >
> > This was covered in the end of the conversation with Paul Moore (that
> > maybe you got tired reading?)
>
> I have not had time to review everything. As I was busy preparing for my
> wedding and am now in the middle of my honeymoon.
I'm very sorry, my bad! You had given me a heads up about this and I
appologise for causing a stir during your special time.
> > I'd originally proposed having it write
> > once, but Paul figured there was no good reason to restrict it and leave
> > that decision up to the orchestrator. The use case would be adding
> > other processes to a container, but it could be argued all additional
> > processes should be spawned by the first process in a container.
>
> I see two cases here:
> a) Nested containers
> b) Inject processes via something like nsenter into a container.
>
> In case a) you have to figure out what to do with nested containers
> and that does seem to be a legitimate case for a double write. Arguably
> with the restriction that you must specify a more nested label.
Is this technically a double write if it is an inheritance? That should
be solvable with a flag.
> In case b) which you seem to be referring to it would be a process
> created by the container manager outside the container that has no
> container label. At which point there is not a need for a double write.
Looking at the potential for nesting, if the orchestrator is already in
a container, then it would already have a label, but if we refer to the
flag solution above, then it is still the first write.
> So my recommendation is to not support double writes until you support
> nested containers.
I think this is a reasonable restriction.
Thanks for your time. Sorry to disturb your holiday.
> Eric
- RGB
--
Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH net] net: systemport: Fix 64-bit statistics dependency
From: David Miller @ 2017-09-19 3:59 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, edumazet, jqiaoulk, kiki-good
In-Reply-To: <20170918233130.5740-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 18 Sep 2017 16:31:30 -0700
> There are several problems with commit 10377ba7673d ("net: systemport:
> Support 64bit statistics", first one got fixed in 7095c973453e ("net:
> systemport: Fix 64-bit stats deadlock").
>
> The second problem is that this specific code updates the
> stats64.tx_{packets,bytes} from ndo_get_stats64() and that is what we
> are returning to ethtool -S. If we are not running a tool that involves
> calling ndo_get_stats64(), then we won't get updated ethtool stats.
>
> The solution to this is to update the stats from both call sites,
> factoring that into a specific function, While at it, don't just check
> the sizeof() but also the type of the statistics in order to use the
> 64-bit stats seqlock.
>
> Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics"
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied, thanks Florian.
^ permalink raw reply
* Re: [PATCH net 0/7] Bug fixes for the HNS3 Ethernet Driver for Hip08 SoC
From: Leon Romanovsky @ 2017-09-19 3:59 UTC (permalink / raw)
To: Salil Mehta
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
lipeng321-hv44wF8Li93QT0dZR+AlfA,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <20170919010628.175732-1-salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]
On Tue, Sep 19, 2017 at 02:06:21AM +0100, Salil Mehta wrote:
> This patch set presents some bug fixes for the HNS3 Ethernet driver, identified
> during internal testing & stabilization efforts.
>
> This patch series is meant for Linux 4.14 kernel.
>
> Lipeng (6):
> net: hns3: get phy addr from NCL_config
> net: hns3: fix the command used to unmap ring from vector
> net: hns3: Fix ring and vector map command
> net: hns3: fix a bug of set mac address
> net: hns3: set default vlan id to PF
> net: hns3: Fixes the premature exit of loop when matching clients
>
> Salil Mehta (1):
> net: hns3: fixes the ether address copy with more appropriate API
1. The fixes patches should have Fixes line and not all of them have
(I didn't look all patches).
2. Please decide on one style: fixes vs. Fixes, fix vs. Fix in the titles
3. Subject should be descriptive and usable, I don't know if it applies
to the "fix a bug of set mac address" patch.
Thanks
>
> drivers/net/ethernet/hisilicon/hns3/hnae3.c | 43 +++++-----------------
> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h | 8 +++-
> .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 20 ++++++++--
> .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 7 ++--
> 4 files changed, 35 insertions(+), 43 deletions(-)
>
> --
> 2.11.0
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net] 8139too: revisit napi_complete_done() usage
From: David Miller @ 2017-09-19 3:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: ville.syrjala, edumazet, netdev
In-Reply-To: <1505765023.29839.32.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 18 Sep 2017 13:03:43 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> It seems we have to be more careful in napi_complete_done()
> use. This patch is not a revert, as it seems we can
> avoid bug that Ville reported by moving the napi_complete_done()
> test in the spinlock section.
>
> Many thanks to Ville for detective work and all tests.
>
> Fixes: 617f01211baf ("8139too: use napi_complete_done()")
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Hangbin Liu @ 2017-09-19 3:05 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, Stephen Hemminger, Phil Sutter
In-Reply-To: <20170918075504.gdx4q7je5uneefgg@unicorn.suse.cz>
Hi Michal,
On Mon, Sep 18, 2017 at 09:55:05AM +0200, Michal Kubecek wrote:
> > +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **answer)
> > +{
> > + struct iovec *iov;
> > + int len = -1, buf_len = 32768;
> > + char *bufp, *buf = NULL;
> > +
> > + int flag = MSG_PEEK | MSG_TRUNC;
> > +
> > +realloc:
> > + bufp = realloc(buf, buf_len);
> > +
> > + if (bufp == NULL) {
> > + fprintf(stderr, "malloc error: not enough buffer\n");
> > + free(buf);
> > + return -ENOMEM;
> > + }
> > + buf = bufp;
> > + iov = msg->msg_iov;
> > + iov->iov_base = buf;
> > + iov->iov_len = buf_len;
> > +
> > +recv:
> > + len = recvmsg(fd, msg, flag);
> > +
> > + if (len < 0) {
> > + if (errno == EINTR || errno == EAGAIN)
> > + goto recv;
> > + fprintf(stderr, "netlink receive error %s (%d)\n",
> > + strerror(errno), errno);
>
> free(buf);
>
> > + return len;
>
> Maybe we should return -errno (saved before calling fprintf()) to be
> consistent.
>
> > + }
> > +
> > + if (len == 0) {
> > + fprintf(stderr, "EOF on netlink\n");
>
> free(buf);
Will fix these three issues.
> > @@ -471,19 +516,23 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> >
> > if (h->nlmsg_type == NLMSG_ERROR) {
> > rtnl_dump_error(rth, h);
> > + free(buf);
> > return -1;
> > }
> >
> > if (!rth->dump_fp) {
> > err = a->filter(&nladdr, h, a->arg1);
> > - if (err < 0)
> > + if (err < 0) {
> > + free(buf);
> > return err;
> > + }
> > }
> >
> > skip_it:
> > h = NLMSG_NEXT(h, msglen);
> > }
> > }
> > + free(buf);
>
> We only free the last buffer returned by rtnl_recvmsg() this way. IMHO
> this free(buf) should be moved inside the loop.
Do you mean the outside while loop or the for loop? I think we could not put
it inside the for loop, because we may need the buf multi times based on arg.
while (1) {
status = rtnl_recvmsg(rth->fd, &msg, &buf);
for (a = arg; a->filter; a++) {
struct nlmsghdr *h = (struct nlmsghdr *)buf;
while (NLMSG_OK(h, msglen)) {
[...]
skip_it:
h = NLMSG_NEXT(h, msglen);
}
}
free(buf);
[...]
}
Thanks
Hangbin
^ permalink raw reply
* Re: RFC: Audit Kernel Container IDs
From: Eric W. Biederman @ 2017-09-19 2:45 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, Linux Containers, Linux API, Linux Audit, Linux FS Devel,
Linux Kernel, Linux Network Development, Aristeu Rozanski,
David Howells, Eric Paris, jlayton, Andy Lutomirski, mszeredi,
Paul Moore, Serge E. Hallyn, Steve Grubb, trondmy, Al Viro
In-Reply-To: <20170914180704.GU3405@madcap2.tricolour.ca>
Richard Guy Briggs <rgb@redhat.com> writes:
> On 2017-09-14 12:33, Eric W. Biederman wrote:
>> Richard Guy Briggs <rgb@redhat.com> writes:
>>
>> > The trigger is a pseudo filesystem (proc, since PID tree already exists)
>> > write of a u64 representing the container ID to a file representing a
>> > process that will become the first process in a new container.
>> > This might place restrictions on mount namespaces required to define a
>> > container, or at least careful checking of namespaces in the kernel to
>> > verify permissions of the orchestrator so it can't change its own
>> > container ID.
>>
>> Why a u64?
>
> u32 will roll too quickly. UUID is large enough that it adds
> significantly to audit record bandwidth. I'd prefer u64, but can look
> at the difference of accommodating a UUID...
I was imagining a string might be better. As for the purposes of audit
it is just a byte string you regurgitate.
>> Why a proc filesystem write and not a magic audit message?
>
> A magic audit message requires CAP_AUDIT_WRITE, which we'd like to use
> sparingly. Given that orchestrators will already require it to send
> the mandatory AUDIT_VIRT_*, this doesn't seem like an unreasonable burden.
>
> I was originally leaning towards an audit message trigger or a syscall.
>
>> I don't like the fact that the proc filesystem entry is likely going to
>> be readable and abusable by non-audit contexts?
>
> This proposal wasn't going to start with that link being readable, but
> its filesystem structure and link names would be, perhaps giving away
> too much already.
>
> I think we will need to find a way for the orchestrator or one of its
> authorized agents to read this information while blocking reads from
> unauthorized agents, otherwise this would be of very limited use.
Something that is set only for future audit messages seems reasonable.
Once you start reading this from something other than audit messages I
get neverous, that people will use this beyond audit for things it is
not intended for.
>> Why the ability to change the containerid? What is the use case you are
>> thinking of there?
>
> This was covered in the end of the conversation with Paul Moore (that
> maybe you got tired reading?)
I have not had time to review everything. As I was busy preparing for my
wedding and am now in the middle of my honeymoon.
> I'd originally proposed having it write
> once, but Paul figured there was no good reason to restrict it and leave
> that decision up to the orchestrator. The use case would be adding
> other processes to a container, but it could be argued all additional
> processes should be spawned by the first process in a container.
I see two cases here:
a) Nested containers
b) Inject processes via something like nsenter into a container.
In case a) you have to figure out what to do with nested containers
and that does seem to be a legitimate case for a double write. Arguably
with the restriction that you must specify a more nested label.
In case b) which you seem to be referring to it would be a process
created by the container manager outside the container that has no
container label. At which point there is not a need for a double write.
So my recommendation is to not support double writes until you support
nested containers.
Eric
^ permalink raw reply
* [PATCH net-next v2 12/12] net: dsa: bcm_sf2: Utilize b53_{enable,disable}_port
From: Florian Fainelli @ 2017-09-19 2:19 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-1-f.fainelli@gmail.com>
Export b53_{enable,disable}_port and use these two functions in
bcm_sf2_port_setup and bcm_sf2_port_disable. The generic functions
cannot be used without wrapping because we need to manage additional
switch integration details (PHY, Broadcom tag etc.).
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 8 ++++----
drivers/net/dsa/b53/b53_priv.h | 2 ++
drivers/net/dsa/bcm_sf2.c | 26 ++------------------------
3 files changed, 8 insertions(+), 28 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index c3f1cd2c33ea..a9f2a5b55a5e 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -502,8 +502,7 @@ void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
}
EXPORT_SYMBOL(b53_imp_vlan_setup);
-static int b53_enable_port(struct dsa_switch *ds, int port,
- struct phy_device *phy)
+int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
{
struct b53_device *dev = ds->priv;
unsigned int cpu_port = dev->cpu_port;
@@ -530,9 +529,9 @@ static int b53_enable_port(struct dsa_switch *ds, int port,
return 0;
}
+EXPORT_SYMBOL(b53_enable_port);
-static void b53_disable_port(struct dsa_switch *ds, int port,
- struct phy_device *phy)
+void b53_disable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
{
struct b53_device *dev = ds->priv;
u8 reg;
@@ -542,6 +541,7 @@ static void b53_disable_port(struct dsa_switch *ds, int port,
reg |= PORT_CTRL_RX_DISABLE | PORT_CTRL_TX_DISABLE;
b53_write8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), reg);
}
+EXPORT_SYMBOL(b53_disable_port);
void b53_brcm_hdr_setup(struct dsa_switch *ds, int port)
{
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index df8bea4105e4..8b5ba78edfd2 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -311,6 +311,8 @@ int b53_mirror_add(struct dsa_switch *ds, int port,
struct dsa_mall_mirror_tc_entry *mirror, bool ingress);
void b53_mirror_del(struct dsa_switch *ds, int port,
struct dsa_mall_mirror_tc_entry *mirror);
+int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy);
+void b53_disable_port(struct dsa_switch *ds, int port, struct phy_device *phy);
void b53_brcm_hdr_setup(struct dsa_switch *ds, int port);
void b53_eee_enable_set(struct dsa_switch *ds, int port, bool enable);
int b53_eee_init(struct dsa_switch *ds, int port, struct phy_device *phy);
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 08639674947a..0072a959db5b 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -163,7 +163,6 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
struct phy_device *phy)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- s8 cpu_port = ds->dst->cpu_dp->index;
unsigned int i;
u32 reg;
@@ -184,9 +183,6 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
reg |= i << (PRT_TO_QID_SHIFT * i);
core_writel(priv, reg, CORE_PORT_TC2_QOS_MAP_PORT(port));
- /* Clear the Rx and Tx disable bits and set to no spanning tree */
- core_writel(priv, 0, CORE_G_PCTL_PORT(port));
-
/* Re-enable the GPHY and re-apply workarounds */
if (priv->int_phy_mask & 1 << port && priv->hw_params.num_gphy == 1) {
bcm_sf2_gphy_enable_set(ds, true);
@@ -209,23 +205,7 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
if (port == priv->moca_port)
bcm_sf2_port_intr_enable(priv, port);
- /* Set this port, and only this one to be in the default VLAN,
- * if member of a bridge, restore its membership prior to
- * bringing down this port.
- */
- reg = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(port));
- reg &= ~PORT_VLAN_CTRL_MASK;
- reg |= (1 << port);
- reg |= priv->dev->ports[port].vlan_ctl_mask;
- core_writel(priv, reg, CORE_PORT_VLAN_CTL_PORT(port));
-
- b53_imp_vlan_setup(ds, cpu_port);
-
- /* If EEE was enabled, restore it */
- if (priv->dev->ports[port].eee.eee_enabled)
- b53_eee_enable_set(ds, port, true);
-
- return 0;
+ return b53_enable_port(ds, port, phy);
}
static void bcm_sf2_port_disable(struct dsa_switch *ds, int port,
@@ -248,9 +228,7 @@ static void bcm_sf2_port_disable(struct dsa_switch *ds, int port,
else
off = CORE_G_PCTL_PORT(port);
- reg = core_readl(priv, off);
- reg |= RX_DIS | TX_DIS;
- core_writel(priv, reg, off);
+ b53_disable_port(ds, port, phy);
/* Power down the port memory */
reg = core_readl(priv, CORE_MEM_PSM_VDD_CTRL);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 07/12] net: dsa: b53: Define EEE register page
From: Florian Fainelli @ 2017-09-19 2:19 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-1-f.fainelli@gmail.com>
In preparation for migrating the EEE code from bcm_sf2 to b53, define the full
EEE register page and offsets within that page.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_regs.h | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index 5e8b8e31fee8..2a9f421680aa 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -50,6 +50,9 @@
/* Jumbo Frame Registers */
#define B53_JUMBO_PAGE 0x40
+/* EEE Control Registers Page */
+#define B53_EEE_PAGE 0x92
+
/* CFP Configuration Registers Page */
#define B53_CFP_PAGE 0xa1
@@ -472,6 +475,44 @@
#define JMS_MAX_SIZE 9724
/*************************************************************************
+ * EEE Configuration Page Registers
+ *************************************************************************/
+
+/* EEE Enable control register (16 bit) */
+#define B53_EEE_EN_CTRL 0x00
+
+/* EEE LPI assert status register (16 bit) */
+#define B53_EEE_LPI_ASSERT_STS 0x02
+
+/* EEE LPI indicate status register (16 bit) */
+#define B53_EEE_LPI_INDICATE 0x4
+
+/* EEE Receiving idle symbols status register (16 bit) */
+#define B53_EEE_RX_IDLE_SYM_STS 0x6
+
+/* EEE Pipeline timer register (32 bit) */
+#define B53_EEE_PIP_TIMER 0xC
+
+/* EEE Sleep timer Gig register (32 bit) */
+#define B53_EEE_SLEEP_TIMER_GIG(i) (0x10 + 4 * (i))
+
+/* EEE Sleep timer FE register (32 bit) */
+#define B53_EEE_SLEEP_TIMER_FE(i) (0x34 + 4 * (i))
+
+/* EEE Minimum LP timer Gig register (32 bit) */
+#define B53_EEE_MIN_LP_TIMER_GIG(i) (0x58 + 4 * (i))
+
+/* EEE Minimum LP timer FE register (32 bit) */
+#define B53_EEE_MIN_LP_TIMER_FE(i) (0x7c + 4 * (i))
+
+/* EEE Wake timer Gig register (16 bit) */
+#define B53_EEE_WAKE_TIMER_GIG(i) (0xa0 + 2 * (i))
+
+/* EEE Wake timer FE register (16 bit) */
+#define B53_EEE_WAKE_TIMER_FE(i) (0xb2 + 2 * (i))
+
+
+/*************************************************************************
* CFP Configuration Page Registers
*************************************************************************/
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 11/12] net: dsa: bcm_sf2: Use SF2_NUM_EGRESS_QUEUES for CFP
From: Florian Fainelli @ 2017-09-19 2:19 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-1-f.fainelli@gmail.com>
The magic number 8 in 3 locations in bcm_sf2_cfp.c actually designates the
number of switch port egress queues, so use that define instead of open-coding
it.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2_cfp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index 8a1da7e67707..94649e1481ec 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -144,7 +144,7 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
* destination port is enabled and that we are within the
* number of ports supported by the switch
*/
- port_num = fs->ring_cookie / 8;
+ port_num = fs->ring_cookie / SF2_NUM_EGRESS_QUEUES;
if (fs->ring_cookie == RX_CLS_FLOW_DISC ||
!(BIT(port_num) & ds->enabled_port_mask) ||
@@ -280,7 +280,7 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
* We have a small oddity where Port 6 just does not have a
* valid bit here (so we subtract by one).
*/
- queue_num = fs->ring_cookie % 8;
+ queue_num = fs->ring_cookie % SF2_NUM_EGRESS_QUEUES;
if (port_num >= 7)
port_num -= 1;
@@ -401,7 +401,7 @@ static int bcm_sf2_cfp_rule_get(struct bcm_sf2_priv *priv, int port,
/* There is no Port 6, so we compensate for that here */
if (nfc->fs.ring_cookie >= 6)
nfc->fs.ring_cookie++;
- nfc->fs.ring_cookie *= 8;
+ nfc->fs.ring_cookie *= SF2_NUM_EGRESS_QUEUES;
/* Extract the destination queue */
queue_num = (reg >> NEW_TC_SHIFT) & NEW_TC_MASK;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 05/12] net: dsa: b53: Use a macro to define I/O operations
From: Florian Fainelli @ 2017-09-19 2:19 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-1-f.fainelli@gmail.com>
Instead of repeating the same pattern: acquire mutex, read/write,
release mutex, define a macro: b53_build_op() which takes the type
(read|write), I/O size, and value (scalar or pointer). This helps with
fixing bugs that could exist (e.g: missing barrier, lock etc.).
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_priv.h | 133 +++++++----------------------------------
1 file changed, 22 insertions(+), 111 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 7528b22aeb03..f1136619e0e4 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -199,119 +199,30 @@ static inline void b53_switch_remove(struct b53_device *dev)
dsa_unregister_switch(dev->ds);
}
-static inline int b53_read8(struct b53_device *dev, u8 page, u8 reg, u8 *val)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->read8(dev, page, reg, val);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int b53_read16(struct b53_device *dev, u8 page, u8 reg, u16 *val)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->read16(dev, page, reg, val);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int b53_read32(struct b53_device *dev, u8 page, u8 reg, u32 *val)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->read32(dev, page, reg, val);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int b53_read48(struct b53_device *dev, u8 page, u8 reg, u64 *val)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->read48(dev, page, reg, val);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
+#define b53_build_op(type, op_size, val_type) \
+static inline int b53_##type##op_size(struct b53_device *dev, u8 page, \
+ u8 reg, val_type val) \
+{ \
+ int ret; \
+ \
+ mutex_lock(&dev->reg_mutex); \
+ ret = dev->ops->type##op_size(dev, page, reg, val); \
+ mutex_unlock(&dev->reg_mutex); \
+ \
+ return ret; \
}
-static inline int b53_read64(struct b53_device *dev, u8 page, u8 reg, u64 *val)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->read64(dev, page, reg, val);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int b53_write8(struct b53_device *dev, u8 page, u8 reg, u8 value)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->write8(dev, page, reg, value);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int b53_write16(struct b53_device *dev, u8 page, u8 reg,
- u16 value)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->write16(dev, page, reg, value);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int b53_write32(struct b53_device *dev, u8 page, u8 reg,
- u32 value)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->write32(dev, page, reg, value);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int b53_write48(struct b53_device *dev, u8 page, u8 reg,
- u64 value)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->write48(dev, page, reg, value);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int b53_write64(struct b53_device *dev, u8 page, u8 reg,
- u64 value)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->write64(dev, page, reg, value);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
+b53_build_op(read, 8, u8 *);
+b53_build_op(read, 16, u16 *);
+b53_build_op(read, 32, u32 *);
+b53_build_op(read, 48, u64 *);
+b53_build_op(read, 64, u64 *);
+
+b53_build_op(write, 8, u8);
+b53_build_op(write, 16, u16);
+b53_build_op(write, 32, u32);
+b53_build_op(write, 48, u64);
+b53_build_op(write, 64, u64);
struct b53_arl_entry {
u8 port;
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 10/12] net: dsa: b53: Export b53_imp_vlan_setup()
From: Florian Fainelli @ 2017-09-19 2:19 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-1-f.fainelli@gmail.com>
bcm_sf2 and b53 do exactly the same thing, so share that piece.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 3 ++-
drivers/net/dsa/b53/b53_priv.h | 1 +
drivers/net/dsa/bcm_sf2.c | 23 +----------------------
3 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 4e37ec27e496..c3f1cd2c33ea 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -484,7 +484,7 @@ static int b53_fast_age_vlan(struct b53_device *dev, u16 vid)
return b53_flush_arl(dev, FAST_AGE_VLAN);
}
-static void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
+void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
{
struct b53_device *dev = ds->priv;
unsigned int i;
@@ -500,6 +500,7 @@ static void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), pvlan);
}
}
+EXPORT_SYMBOL(b53_imp_vlan_setup);
static int b53_enable_port(struct dsa_switch *ds, int port,
struct phy_device *phy)
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 0ed59672ef07..df8bea4105e4 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -284,6 +284,7 @@ static inline int b53_switch_get_reset_gpio(struct b53_device *dev)
#endif
/* Exported functions towards other drivers */
+void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port);
void b53_get_strings(struct dsa_switch *ds, int port, uint8_t *data);
void b53_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data);
int b53_get_sset_count(struct dsa_switch *ds);
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 4e8ef4c07eab..08639674947a 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -40,27 +40,6 @@ static enum dsa_tag_protocol bcm_sf2_sw_get_tag_protocol(struct dsa_switch *ds)
return DSA_TAG_PROTO_BRCM;
}
-static void bcm_sf2_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
-{
- struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
- unsigned int i;
- u32 reg;
-
- /* Enable the IMP Port to be in the same VLAN as the other ports
- * on a per-port basis such that we only have Port i and IMP in
- * the same VLAN.
- */
- for (i = 0; i < priv->hw_params.num_ports; i++) {
- if (!((1 << i) & ds->enabled_port_mask))
- continue;
-
- reg = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(i));
- reg |= (1 << cpu_port);
- core_writel(priv, reg, CORE_PORT_VLAN_CTL_PORT(i));
- }
-}
-
-
static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
@@ -240,7 +219,7 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
reg |= priv->dev->ports[port].vlan_ctl_mask;
core_writel(priv, reg, CORE_PORT_VLAN_CTL_PORT(port));
- bcm_sf2_imp_vlan_setup(ds, cpu_port);
+ b53_imp_vlan_setup(ds, cpu_port);
/* If EEE was enabled, restore it */
if (priv->dev->ports[port].eee.eee_enabled)
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 06/12] net: dsa: b53: Move Broadcom header setup to b53
From: Florian Fainelli @ 2017-09-19 2:19 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-1-f.fainelli@gmail.com>
The code to enable Broadcom tags/headers is largely switch independent,
and in preparation for enabling it for multiple devices with b53, move
the code we have in bcm_sf2.c to b53_common.c
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 47 ++++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/b53/b53_priv.h | 1 +
drivers/net/dsa/b53/b53_regs.h | 7 ++++++
drivers/net/dsa/bcm_sf2.c | 43 ++----------------------------------
drivers/net/dsa/bcm_sf2_regs.h | 8 -------
5 files changed, 57 insertions(+), 49 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 3297af6aab8a..aa2187c71ea5 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -538,6 +538,53 @@ static void b53_disable_port(struct dsa_switch *ds, int port,
b53_write8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), reg);
}
+void b53_brcm_hdr_setup(struct dsa_switch *ds, int port)
+{
+ struct b53_device *dev = ds->priv;
+ u8 hdr_ctl, val;
+ u16 reg;
+
+ /* Resolve which bit controls the Broadcom tag */
+ switch (port) {
+ case 8:
+ val = BRCM_HDR_P8_EN;
+ break;
+ case 7:
+ val = BRCM_HDR_P7_EN;
+ break;
+ case 5:
+ val = BRCM_HDR_P5_EN;
+ break;
+ default:
+ val = 0;
+ break;
+ }
+
+ /* Enable Broadcom tags for IMP port */
+ b53_read8(dev, B53_MGMT_PAGE, B53_BRCM_HDR, &hdr_ctl);
+ hdr_ctl |= val;
+ b53_write8(dev, B53_MGMT_PAGE, B53_BRCM_HDR, hdr_ctl);
+
+ /* Registers below are only accessible on newer devices */
+ if (!is58xx(dev))
+ return;
+
+ /* Enable reception Broadcom tag for CPU TX (switch RX) to
+ * allow us to tag outgoing frames
+ */
+ b53_read16(dev, B53_MGMT_PAGE, B53_BRCM_HDR_RX_DIS, ®);
+ reg &= ~BIT(port);
+ b53_write16(dev, B53_MGMT_PAGE, B53_BRCM_HDR_RX_DIS, reg);
+
+ /* Enable transmission of Broadcom tags from the switch (CPU RX) to
+ * allow delivering frames to the per-port net_devices
+ */
+ b53_read16(dev, B53_MGMT_PAGE, B53_BRCM_HDR_TX_DIS, ®);
+ reg &= ~BIT(port);
+ b53_write16(dev, B53_MGMT_PAGE, B53_BRCM_HDR_TX_DIS, reg);
+}
+EXPORT_SYMBOL(b53_brcm_hdr_setup);
+
static void b53_enable_cpu_port(struct b53_device *dev, int port)
{
u8 port_ctrl;
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index f1136619e0e4..44297b7c3795 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -309,5 +309,6 @@ int b53_mirror_add(struct dsa_switch *ds, int port,
struct dsa_mall_mirror_tc_entry *mirror, bool ingress);
void b53_mirror_del(struct dsa_switch *ds, int port,
struct dsa_mall_mirror_tc_entry *mirror);
+void b53_brcm_hdr_setup(struct dsa_switch *ds, int port);
#endif
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index e5c86d44667a..5e8b8e31fee8 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -210,6 +210,7 @@
#define B53_BRCM_HDR 0x03
#define BRCM_HDR_P8_EN BIT(0) /* Enable tagging on port 8 */
#define BRCM_HDR_P5_EN BIT(1) /* Enable tagging on port 5 */
+#define BRCM_HDR_P7_EN BIT(2) /* Enable tagging on port 7 */
/* Mirror capture control register (16 bit) */
#define B53_MIR_CAP_CTL 0x10
@@ -249,6 +250,12 @@
/* Revision ID register (8 bit) */
#define B53_REV_ID 0x40
+/* Broadcom header RX control (16 bit) */
+#define B53_BRCM_HDR_RX_DIS 0x60
+
+/* Broadcom header TX control (16 bit) */
+#define B53_BRCM_HDR_TX_DIS 0x62
+
/*************************************************************************
* ARL Access Page Registers
*************************************************************************/
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 8acbd17bc1fd..49cb51223f70 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -60,45 +60,6 @@ static void bcm_sf2_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
}
}
-static void bcm_sf2_brcm_hdr_setup(struct bcm_sf2_priv *priv, int port)
-{
- u32 reg, val;
-
- /* Resolve which bit controls the Broadcom tag */
- switch (port) {
- case 8:
- val = BRCM_HDR_EN_P8;
- break;
- case 7:
- val = BRCM_HDR_EN_P7;
- break;
- case 5:
- val = BRCM_HDR_EN_P5;
- break;
- default:
- val = 0;
- break;
- }
-
- /* Enable Broadcom tags for IMP port */
- reg = core_readl(priv, CORE_BRCM_HDR_CTRL);
- reg |= val;
- core_writel(priv, reg, CORE_BRCM_HDR_CTRL);
-
- /* Enable reception Broadcom tag for CPU TX (switch RX) to
- * allow us to tag outgoing frames
- */
- reg = core_readl(priv, CORE_BRCM_HDR_RX_DIS);
- reg &= ~(1 << port);
- core_writel(priv, reg, CORE_BRCM_HDR_RX_DIS);
-
- /* Enable transmission of Broadcom tags from the switch (CPU RX) to
- * allow delivering frames to the per-port net_devices
- */
- reg = core_readl(priv, CORE_BRCM_HDR_TX_DIS);
- reg &= ~(1 << port);
- core_writel(priv, reg, CORE_BRCM_HDR_TX_DIS);
-}
static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
{
@@ -138,7 +99,7 @@ static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
reg |= i << (PRT_TO_QID_SHIFT * i);
core_writel(priv, reg, CORE_PORT_TC2_QOS_MAP_PORT(port));
- bcm_sf2_brcm_hdr_setup(priv, port);
+ b53_brcm_hdr_setup(ds, port);
/* Force link status for IMP port */
reg = core_readl(priv, offset);
@@ -247,7 +208,7 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
/* Enable Broadcom tags for that port if requested */
if (priv->brcm_tag_mask & BIT(port))
- bcm_sf2_brcm_hdr_setup(priv, port);
+ b53_brcm_hdr_setup(ds, port);
/* Configure Traffic Class to QoS mapping, allow each priority to map
* to a different queue number
diff --git a/drivers/net/dsa/bcm_sf2_regs.h b/drivers/net/dsa/bcm_sf2_regs.h
index 49695fcc2ea8..788361ad68a0 100644
--- a/drivers/net/dsa/bcm_sf2_regs.h
+++ b/drivers/net/dsa/bcm_sf2_regs.h
@@ -205,16 +205,8 @@ enum bcm_sf2_reg_offs {
#define CORE_IMP0_PRT_ID 0x0804
-#define CORE_BRCM_HDR_CTRL 0x0080c
-#define BRCM_HDR_EN_P8 (1 << 0)
-#define BRCM_HDR_EN_P5 (1 << 1)
-#define BRCM_HDR_EN_P7 (1 << 2)
-
#define CORE_RST_MIB_CNT_EN 0x0950
-#define CORE_BRCM_HDR_RX_DIS 0x0980
-#define CORE_BRCM_HDR_TX_DIS 0x0988
-
#define CORE_ARLA_VTBL_RWCTRL 0x1600
#define ARLA_VTBL_CMD_WRITE 0
#define ARLA_VTBL_CMD_READ 1
--
2.9.3
^ permalink raw reply related
* [PATCH net-next v2 09/12] net: dsa: b53: Wire-up EEE
From: Florian Fainelli @ 2017-09-19 2:19 UTC (permalink / raw)
To: netdev; +Cc: andrew, vivien.didelot, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-1-f.fainelli@gmail.com>
Add support for enabling and disabling EEE, as well as re-negotiating it in
.adjust_link() and in .port_enable().
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 491e4ffa8a0e..4e37ec27e496 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -523,6 +523,10 @@ static int b53_enable_port(struct dsa_switch *ds, int port,
b53_imp_vlan_setup(ds, cpu_port);
+ /* If EEE was enabled, restore it */
+ if (dev->ports[port].eee.eee_enabled)
+ b53_eee_enable_set(ds, port, true);
+
return 0;
}
@@ -879,6 +883,7 @@ static void b53_adjust_link(struct dsa_switch *ds, int port,
struct phy_device *phydev)
{
struct b53_device *dev = ds->priv;
+ struct ethtool_eee *p = &dev->ports[port].eee;
u8 rgmii_ctrl = 0, reg = 0, off;
if (!phy_is_pseudo_fixed_link(phydev))
@@ -1000,6 +1005,9 @@ static void b53_adjust_link(struct dsa_switch *ds, int port,
b53_write8(dev, B53_CTRL_PAGE, po_reg, gmii_po);
}
}
+
+ /* Re-negotiate EEE if it was enabled already */
+ p->eee_enabled = b53_eee_init(ds, port, phydev);
}
int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
@@ -1605,6 +1613,8 @@ static const struct dsa_switch_ops b53_switch_ops = {
.adjust_link = b53_adjust_link,
.port_enable = b53_enable_port,
.port_disable = b53_disable_port,
+ .get_mac_eee = b53_get_mac_eee,
+ .set_mac_eee = b53_set_mac_eee,
.port_bridge_join = b53_br_join,
.port_bridge_leave = b53_br_leave,
.port_stp_state_set = b53_br_set_stp_state,
--
2.9.3
^ permalink raw reply related
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