* Re: [PATCH v2 1/9] arm64: dts: rockchip: add eMMC's power domain support for rk3399
From: Caesar Wang @ 2016-11-14 15:05 UTC (permalink / raw)
To: Shawn Lin
Cc: Caesar Wang, Heiko Stuebner, Mark Rutland, devicetree,
Brian Norris, linux-kernel, Catalin Marinas, Elaine Zhang,
Will Deacon, Douglas Anderson, tfiga, linux-rockchip, Rob Herring,
eddie.cai, David Wu, Jianqun Xu, linux-arm-kernel, Ziyuan Xu
In-Reply-To: <bce1a5eb-fbb3-9ed9-46b9-72a95eefed3e@rock-chips.com>
On 2016年11月12日 12:22, Shawn Lin wrote:
> On 2016/11/9 21:21, Caesar Wang wrote:
>> From: Ziyuan Xu <xzy.xu@rock-chips.com>
>>
>> Control power domain for eMMC via genpd to reduce power consumption.
>>
>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>
> It was verified on my rk3399 evb with kernel4.4, so
> free feel to add my tag,
>
> Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
>
Thanks the tests.
> BTW, it seems my reply is bounced form Yakir's address, so please
> remove him from CC list if he changed his mail address.
Right, Yakir's (ykk@rock-chips.com) address had left this world.;-)
But the patman tool is auto sending with the Cc people.
-
Caesar
>
>> ---
>>
>> Changes in v2:
>> - Reviewed-on: https://chromium-review.googlesource.com/376558
>> - Verified on ChromeOS kernel4.4
>>
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index cbb7f8b..b401176 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -269,6 +269,7 @@
>> #clock-cells = <0>;
>> phys = <&emmc_phy>;
>> phy-names = "phy_arasan";
>> + power-domains = <&power RK3399_PD_EMMC>;
>> status = "disabled";
>> };
>>
>> @@ -690,6 +691,11 @@
>> status = "disabled";
>> };
>>
>> + qos_emmc: qos@ffa58000 {
>> + compatible = "syscon";
>> + reg = <0x0 0xffa58000 0x0 0x20>;
>> + };
>> +
>> qos_gmac: qos@ffa5c000 {
>> compatible = "syscon";
>> reg = <0x0 0xffa5c000 0x0 0x20>;
>> @@ -823,6 +829,11 @@
>> };
>>
>> /* These power domains are grouped by VD_LOGIC */
>> + pd_emmc@RK3399_PD_EMMC {
>> + reg = <RK3399_PD_EMMC>;
>> + clocks = <&cru ACLK_EMMC>;
>> + pm_qos = <&qos_emmc>;
>> + };
>> pd_gmac@RK3399_PD_GMAC {
>> reg = <RK3399_PD_GMAC>;
>> clocks = <&cru ACLK_GMAC>;
>>
>
>
--
caesar wang | software engineer | wxt@rock-chip.com
^ permalink raw reply
* [PATCH v2 1/9] arm64: dts: rockchip: add eMMC's power domain support for rk3399
From: Caesar Wang @ 2016-11-14 15:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bce1a5eb-fbb3-9ed9-46b9-72a95eefed3e@rock-chips.com>
On 2016?11?12? 12:22, Shawn Lin wrote:
> On 2016/11/9 21:21, Caesar Wang wrote:
>> From: Ziyuan Xu <xzy.xu@rock-chips.com>
>>
>> Control power domain for eMMC via genpd to reduce power consumption.
>>
>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>
> It was verified on my rk3399 evb with kernel4.4, so
> free feel to add my tag,
>
> Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
>
Thanks the tests.
> BTW, it seems my reply is bounced form Yakir's address, so please
> remove him from CC list if he changed his mail address.
Right, Yakir's (ykk at rock-chips.com) address had left this world.;-)
But the patman tool is auto sending with the Cc people.
-
Caesar
>
>> ---
>>
>> Changes in v2:
>> - Reviewed-on: https://chromium-review.googlesource.com/376558
>> - Verified on ChromeOS kernel4.4
>>
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index cbb7f8b..b401176 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -269,6 +269,7 @@
>> #clock-cells = <0>;
>> phys = <&emmc_phy>;
>> phy-names = "phy_arasan";
>> + power-domains = <&power RK3399_PD_EMMC>;
>> status = "disabled";
>> };
>>
>> @@ -690,6 +691,11 @@
>> status = "disabled";
>> };
>>
>> + qos_emmc: qos at ffa58000 {
>> + compatible = "syscon";
>> + reg = <0x0 0xffa58000 0x0 0x20>;
>> + };
>> +
>> qos_gmac: qos at ffa5c000 {
>> compatible = "syscon";
>> reg = <0x0 0xffa5c000 0x0 0x20>;
>> @@ -823,6 +829,11 @@
>> };
>>
>> /* These power domains are grouped by VD_LOGIC */
>> + pd_emmc at RK3399_PD_EMMC {
>> + reg = <RK3399_PD_EMMC>;
>> + clocks = <&cru ACLK_EMMC>;
>> + pm_qos = <&qos_emmc>;
>> + };
>> pd_gmac at RK3399_PD_GMAC {
>> reg = <RK3399_PD_GMAC>;
>> clocks = <&cru ACLK_GMAC>;
>>
>
>
--
caesar wang | software engineer | wxt at rock-chip.com
^ permalink raw reply
* Re: [PATCH v2 1/9] arm64: dts: rockchip: add eMMC's power domain support for rk3399
From: Caesar Wang @ 2016-11-14 15:05 UTC (permalink / raw)
To: Shawn Lin
Cc: Caesar Wang, Heiko Stuebner, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Brian Norris,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas,
Elaine Zhang, Will Deacon, Douglas Anderson,
tfiga-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
eddie.cai-TNX95d0MmH7DzftRWevZcw, David Wu, Jianqun Xu,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Ziyuan Xu
In-Reply-To: <bce1a5eb-fbb3-9ed9-46b9-72a95eefed3e-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On 2016年11月12日 12:22, Shawn Lin wrote:
> On 2016/11/9 21:21, Caesar Wang wrote:
>> From: Ziyuan Xu <xzy.xu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>
>> Control power domain for eMMC via genpd to reduce power consumption.
>>
>> Signed-off-by: Elaine Zhang <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> Signed-off-by: Ziyuan Xu <xzy.xu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> It was verified on my rk3399 evb with kernel4.4, so
> free feel to add my tag,
>
> Tested-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
Thanks the tests.
> BTW, it seems my reply is bounced form Yakir's address, so please
> remove him from CC list if he changed his mail address.
Right, Yakir's (ykk-TNX95d0MmH7DzftRWevZcw@public.gmane.org) address had left this world.;-)
But the patman tool is auto sending with the Cc people.
-
Caesar
>
>> ---
>>
>> Changes in v2:
>> - Reviewed-on: https://chromium-review.googlesource.com/376558
>> - Verified on ChromeOS kernel4.4
>>
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index cbb7f8b..b401176 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -269,6 +269,7 @@
>> #clock-cells = <0>;
>> phys = <&emmc_phy>;
>> phy-names = "phy_arasan";
>> + power-domains = <&power RK3399_PD_EMMC>;
>> status = "disabled";
>> };
>>
>> @@ -690,6 +691,11 @@
>> status = "disabled";
>> };
>>
>> + qos_emmc: qos@ffa58000 {
>> + compatible = "syscon";
>> + reg = <0x0 0xffa58000 0x0 0x20>;
>> + };
>> +
>> qos_gmac: qos@ffa5c000 {
>> compatible = "syscon";
>> reg = <0x0 0xffa5c000 0x0 0x20>;
>> @@ -823,6 +829,11 @@
>> };
>>
>> /* These power domains are grouped by VD_LOGIC */
>> + pd_emmc@RK3399_PD_EMMC {
>> + reg = <RK3399_PD_EMMC>;
>> + clocks = <&cru ACLK_EMMC>;
>> + pm_qos = <&qos_emmc>;
>> + };
>> pd_gmac@RK3399_PD_GMAC {
>> reg = <RK3399_PD_GMAC>;
>> clocks = <&cru ACLK_GMAC>;
>>
>
>
--
caesar wang | software engineer | wxt-TNX95d0MmH73oGB3hsPCZA@public.gmane.org
--
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: [Qemu-devel] [PATCH v7 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support
From: Stefan Hajnoczi @ 2016-11-14 15:05 UTC (permalink / raw)
To: Ashish Mittal
Cc: qemu-devel, Paolo Bonzini, Kevin Wolf, Markus Armbruster,
Daniel P. Berrange, Jeff Cody, Fam Zheng, Ashish Mittal,
Ketan Nilangekar, Abhijit Dey
In-Reply-To: <20160928214510.GA2837@stefanha-x1.localdomain>
On Wed, Sep 28, 2016 at 10:45 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Sep 27, 2016 at 09:09:49PM -0700, Ashish Mittal wrote:
>
> Review of .bdrv_open() and .bdrv_aio_writev() code paths.
>
> The big issues I see in this driver and libqnio:
>
> 1. Showstoppers like broken .bdrv_open() and leaking memory on every
> reply message.
> 2. Insecure due to missing input validation (network packets and
> configuration) and incorrect string handling.
> 3. Not fully asynchronous so QEMU and the guest may hang.
>
> Please think about the whole codebase and not just the lines I've
> pointed out in this review when fixing these sorts of issues. There may
> be similar instances of these bugs elsewhere and it's important that
> they are fixed so that this can be merged.
Ping?
You didn't respond to the comments I raised. The libqnio buffer
overflows and other issues from this email are still present.
I put a lot of time into reviewing this patch series and libqnio. If
you want to get reviews please address feedback before sending a new
patch series.
>
>> +/*
>> + * Structure per vDisk maintained for state
>> + */
>> +typedef struct BDRVVXHSState {
>> + int fds[2];
>> + int64_t vdisk_size;
>> + int64_t vdisk_blocks;
>> + int64_t vdisk_flags;
>> + int vdisk_aio_count;
>> + int event_reader_pos;
>> + VXHSAIOCB *qnio_event_acb;
>> + void *qnio_ctx;
>> + QemuSpin vdisk_lock; /* Lock to protect BDRVVXHSState */
>> + QemuSpin vdisk_acb_lock; /* Protects ACB */
>
> These comments are insufficient for documenting locking. Not all fields
> are actually protected by these locks. Please order fields according to
> lock coverage:
>
> typedef struct VXHSAIOCB {
> ...
>
> /* Protected by BDRVVXHSState->vdisk_acb_lock */
> int segments;
> ...
> };
>
> typedef struct BDRVVXHSState {
> ...
>
> /* Protected by vdisk_lock */
> QemuSpin vdisk_lock;
> int vdisk_aio_count;
> QSIMPLEQ_HEAD(aio_retryq, VXHSAIOCB) vdisk_aio_retryq;
> ...
> }
>
>> +static void vxhs_qnio_iio_close(BDRVVXHSState *s, int idx)
>> +{
>> + /*
>> + * Close vDisk device
>> + */
>> + if (s->vdisk_hostinfo[idx].vdisk_rfd >= 0) {
>> + iio_devclose(s->qnio_ctx, 0, s->vdisk_hostinfo[idx].vdisk_rfd);
>
> libqnio comment:
> Why does iio_devclose() take an unused cfd argument? Perhaps it can be
> dropped.
>
>> + s->vdisk_hostinfo[idx].vdisk_rfd = -1;
>> + }
>> +
>> + /*
>> + * Close QNIO channel against cached channel-fd
>> + */
>> + if (s->vdisk_hostinfo[idx].qnio_cfd >= 0) {
>> + iio_close(s->qnio_ctx, s->vdisk_hostinfo[idx].qnio_cfd);
>
> libqnio comment:
> Why does iio_devclose() take an int32_t cfd argument but iio_close()
> takes a uint32_t cfd argument?
>
>> + s->vdisk_hostinfo[idx].qnio_cfd = -1;
>> + }
>> +}
>> +
>> +static int vxhs_qnio_iio_open(int *cfd, const char *of_vsa_addr,
>> + int *rfd, const char *file_name)
>> +{
>> + /*
>> + * Open qnio channel to storage agent if not opened before.
>> + */
>> + if (*cfd < 0) {
>> + *cfd = iio_open(global_qnio_ctx, of_vsa_addr, 0);
>
> libqnio comments:
>
> 1.
> There is a buffer overflow in qnio_create_channel(). strncpy() is used
> incorrectly so long hostname or port (both can be 99 characters long)
> will overflow channel->name[] (64 characters) or channel->port[] (8
> characters).
>
> strncpy(channel->name, hostname, strlen(hostname) + 1);
> strncpy(channel->port, port, strlen(port) + 1);
>
> The third argument must be the size of the *destination* buffer, not the
> source buffer. Also note that strncpy() doesn't NUL-terminate the
> destination string so you must do that manually to ensure there is a NUL
> byte at the end of the buffer.
>
> 2.
> channel is leaked in the "Failed to open single connection" error case
> in qnio_create_channel().
>
> 3.
> If host is longer the 63 characters then the ioapi_ctx->channels and
> qnio_ctx->channels maps will use different keys due to string truncation
> in qnio_create_channel(). This means "Channel already exists" in
> qnio_create_channel() and possibly other things will not work as
> expected.
>
>> + if (*cfd < 0) {
>> + trace_vxhs_qnio_iio_open(of_vsa_addr);
>> + return -ENODEV;
>> + }
>> + }
>> +
>> + /*
>> + * Open vdisk device
>> + */
>> + *rfd = iio_devopen(global_qnio_ctx, *cfd, file_name, 0);
>
> libqnio comment:
> Buffer overflow in iio_devopen() since chandev[128] is not large enough
> to hold channel[100] + " " + devpath[arbitrary length] chars:
>
> sprintf(chandev, "%s %s", channel, devpath);
>
>> +
>> + if (*rfd < 0) {
>> + if (*cfd >= 0) {
>
> This check is always true. Otherwise the return -ENODEV would have been
> taken above. The if statement isn't necessary.
>
>> +static void vxhs_check_failover_status(int res, void *ctx)
>> +{
>> + BDRVVXHSState *s = ctx;
>> +
>> + if (res == 0) {
>> + /* found failover target */
>> + s->vdisk_cur_host_idx = s->vdisk_ask_failover_idx;
>> + s->vdisk_ask_failover_idx = 0;
>> + trace_vxhs_check_failover_status(
>> + s->vdisk_hostinfo[s->vdisk_cur_host_idx].hostip,
>> + s->vdisk_guid);
>> + qemu_spin_lock(&s->vdisk_lock);
>> + OF_VDISK_RESET_IOFAILOVER_IN_PROGRESS(s);
>> + qemu_spin_unlock(&s->vdisk_lock);
>> + vxhs_handle_queued_ios(s);
>> + } else {
>> + /* keep looking */
>> + trace_vxhs_check_failover_status_retry(s->vdisk_guid);
>> + s->vdisk_ask_failover_idx++;
>> + if (s->vdisk_ask_failover_idx == s->vdisk_nhosts) {
>> + /* pause and cycle through list again */
>> + sleep(QNIO_CONNECT_RETRY_SECS);
>
> This code is called from a QEMU thread via vxhs_aio_rw(). It is not
> permitted to call sleep() since it will freeze QEMU and probably the
> guest.
>
> If you need a timer you can use QEMU's timer APIs. See aio_timer_new(),
> timer_new_ns(), timer_mod(), timer_del(), timer_free().
>
>> + s->vdisk_ask_failover_idx = 0;
>> + }
>> + res = vxhs_switch_storage_agent(s);
>> + }
>> +}
>> +
>> +static int vxhs_failover_io(BDRVVXHSState *s)
>> +{
>> + int res = 0;
>> +
>> + trace_vxhs_failover_io(s->vdisk_guid);
>> +
>> + s->vdisk_ask_failover_idx = 0;
>> + res = vxhs_switch_storage_agent(s);
>> +
>> + return res;
>> +}
>> +
>> +static void vxhs_iio_callback(int32_t rfd, uint32_t reason, void *ctx,
>> + uint32_t error, uint32_t opcode)
>
> This function is doing too much. Especially the failover code should
> run in the AioContext since it's complex. Don't do failover here
> because this function is outside the AioContext lock. Do it from
> AioContext using a QEMUBH like block/rbd.c.
>
>> +static int32_t
>> +vxhs_qnio_iio_writev(void *qnio_ctx, uint32_t rfd, QEMUIOVector *qiov,
>> + uint64_t offset, void *ctx, uint32_t flags)
>> +{
>> + struct iovec cur;
>> + uint64_t cur_offset = 0;
>> + uint64_t cur_write_len = 0;
>> + int segcount = 0;
>> + int ret = 0;
>> + int i, nsio = 0;
>> + int iovcnt = qiov->niov;
>> + struct iovec *iov = qiov->iov;
>> +
>> + errno = 0;
>> + cur.iov_base = 0;
>> + cur.iov_len = 0;
>> +
>> + ret = iio_writev(qnio_ctx, rfd, iov, iovcnt, offset, ctx, flags);
>
> libqnio comments:
>
> 1.
> There are blocking connect(2) and getaddrinfo(3) calls in iio_writev()
> so this may hang for arbitrary amounts of time. This is not permitted
> in .bdrv_aio_readv()/.bdrv_aio_writev(). Please make qnio actually
> asynchronous.
>
> 2.
> Where does client_callback() free reply? It looks like every reply
> message causes a memory leak!
>
> 3.
> Buffer overflow in iio_writev() since device[128] cannot fit the device
> string generated from the vdisk_guid.
>
> 4.
> Buffer overflow in iio_writev() due to
> strncpy(msg->hinfo.target,device,strlen(device)) where device[128] is
> larger than target[64]. Also note the previous comments about strncpy()
> usage.
>
> 5.
> I don't see any endianness handling or portable alignment of struct
> fields in the network protocol code. Binary network protocols need to
> take care of these issue for portability. This means libqnio compiled
> for different architectures will not work. Do you plan to support any
> other architectures besides x86?
>
> 6.
> The networking code doesn't look robust: kvset uses assert() on input
> from the network so the other side of the connection could cause SIGABRT
> (coredump), the client uses the msg pointer as the cookie for the
> response packet so the server can easily crash the client by sending a
> bogus cookie value, etc. Even on the client side these things are
> troublesome but on a server they are guaranteed security issues. I
> didn't look into it deeply. Please audit the code.
>
>> +static int vxhs_qemu_init(QDict *options, BDRVVXHSState *s,
>> + int *cfd, int *rfd, Error **errp)
>> +{
>> + QDict *backing_options = NULL;
>> + QemuOpts *opts, *tcp_opts;
>> + const char *vxhs_filename;
>> + char *of_vsa_addr = NULL;
>> + Error *local_err = NULL;
>> + const char *vdisk_id_opt;
>> + char *file_name = NULL;
>> + size_t num_servers = 0;
>> + char *str = NULL;
>> + int ret = 0;
>> + int i;
>> +
>> + opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
>> + qemu_opts_absorb_qdict(opts, options, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + vxhs_filename = qemu_opt_get(opts, VXHS_OPT_FILENAME);
>> + if (vxhs_filename) {
>> + trace_vxhs_qemu_init_filename(vxhs_filename);
>> + }
>> +
>> + vdisk_id_opt = qemu_opt_get(opts, VXHS_OPT_VDISK_ID);
>> + if (!vdisk_id_opt) {
>> + error_setg(&local_err, QERR_MISSING_PARAMETER, VXHS_OPT_VDISK_ID);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + s->vdisk_guid = g_strdup(vdisk_id_opt);
>> + trace_vxhs_qemu_init_vdisk(vdisk_id_opt);
>> +
>> + num_servers = qdict_array_entries(options, VXHS_OPT_SERVER);
>> + if (num_servers < 1) {
>> + error_setg(&local_err, QERR_MISSING_PARAMETER, "server");
>> + ret = -EINVAL;
>> + goto out;
>> + } else if (num_servers > VXHS_MAX_HOSTS) {
>> + error_setg(&local_err, QERR_INVALID_PARAMETER, "server");
>> + error_append_hint(errp, "Maximum %d servers allowed.\n",
>> + VXHS_MAX_HOSTS);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + trace_vxhs_qemu_init_numservers(num_servers);
>> +
>> + for (i = 0; i < num_servers; i++) {
>> + str = g_strdup_printf(VXHS_OPT_SERVER"%d.", i);
>> + qdict_extract_subqdict(options, &backing_options, str);
>> +
>> + /* Create opts info from runtime_tcp_opts list */
>> + tcp_opts = qemu_opts_create(&runtime_tcp_opts, NULL, 0, &error_abort);
>> + qemu_opts_absorb_qdict(tcp_opts, backing_options, &local_err);
>> + if (local_err) {
>> + qdict_del(backing_options, str);
>
> backing_options is leaked and there's no need to delete the str key.
>
>> + qemu_opts_del(tcp_opts);
>> + g_free(str);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + s->vdisk_hostinfo[i].hostip = g_strdup(qemu_opt_get(tcp_opts,
>> + VXHS_OPT_HOST));
>> + s->vdisk_hostinfo[i].port = g_ascii_strtoll(qemu_opt_get(tcp_opts,
>> + VXHS_OPT_PORT),
>> + NULL, 0);
>
> This will segfault if the port option was missing.
>
>> +
>> + s->vdisk_hostinfo[i].qnio_cfd = -1;
>> + s->vdisk_hostinfo[i].vdisk_rfd = -1;
>> + trace_vxhs_qemu_init(s->vdisk_hostinfo[i].hostip,
>> + s->vdisk_hostinfo[i].port);
>
> It's not safe to use the %s format specifier for a trace event with a
> NULL value. In the case where hostip is NULL this could crash on some
> systems.
>
>> +
>> + qdict_del(backing_options, str);
>> + qemu_opts_del(tcp_opts);
>> + g_free(str);
>> + }
>
> backing_options is leaked.
>
>> +
>> + s->vdisk_nhosts = i;
>> + s->vdisk_cur_host_idx = 0;
>> + file_name = g_strdup_printf("%s%s", vdisk_prefix, s->vdisk_guid);
>> + of_vsa_addr = g_strdup_printf("of://%s:%d",
>> + s->vdisk_hostinfo[s->vdisk_cur_host_idx].hostip,
>> + s->vdisk_hostinfo[s->vdisk_cur_host_idx].port);
>
> Can we get here with num_servers == 0? In that case this would access
> uninitialized memory. I guess num_servers == 0 does not make sense and
> there should be an error case for it.
>
>> +
>> + /*
>> + * .bdrv_open() and .bdrv_create() run under the QEMU global mutex.
>> + */
>> + if (global_qnio_ctx == NULL) {
>> + global_qnio_ctx = vxhs_setup_qnio();
>
> libqnio comment:
> The client epoll thread should mask all signals (like
> qemu_thread_create()). Otherwise it may receive signals that it cannot
> deal with.
>
>> + if (global_qnio_ctx == NULL) {
>> + error_setg(&local_err, "Failed vxhs_setup_qnio");
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + }
>> +
>> + ret = vxhs_qnio_iio_open(cfd, of_vsa_addr, rfd, file_name);
>> + if (!ret) {
>> + error_setg(&local_err, "Failed qnio_iio_open");
>> + ret = -EIO;
>> + }
>
> The return value of vxhs_qnio_iio_open() is 0 for success or -errno for
> error.
>
> I guess you never ran this code! The block driver won't even open
> successfully.
>
>> +
>> +out:
>> + g_free(file_name);
>> + g_free(of_vsa_addr);
>> + qemu_opts_del(opts);
>> +
>> + if (ret < 0) {
>> + for (i = 0; i < num_servers; i++) {
>> + g_free(s->vdisk_hostinfo[i].hostip);
>> + }
>> + g_free(s->vdisk_guid);
>> + s->vdisk_guid = NULL;
>> + errno = -ret;
>
> There is no need to set errno here. The return value already contains
> the error and the caller doesn't look at errno.
>
>> + }
>> + error_propagate(errp, local_err);
>> +
>> + return ret;
>> +}
>> +
>> +static int vxhs_open(BlockDriverState *bs, QDict *options,
>> + int bdrv_flags, Error **errp)
>> +{
>> + BDRVVXHSState *s = bs->opaque;
>> + AioContext *aio_context;
>> + int qemu_qnio_cfd = -1;
>> + int device_opened = 0;
>> + int qemu_rfd = -1;
>> + int ret = 0;
>> + int i;
>> +
>> + ret = vxhs_qemu_init(options, s, &qemu_qnio_cfd, &qemu_rfd, errp);
>> + if (ret < 0) {
>> + trace_vxhs_open_fail(ret);
>> + return ret;
>> + }
>> +
>> + device_opened = 1;
>> + s->qnio_ctx = global_qnio_ctx;
>> + s->vdisk_hostinfo[0].qnio_cfd = qemu_qnio_cfd;
>> + s->vdisk_hostinfo[0].vdisk_rfd = qemu_rfd;
>> + s->vdisk_size = 0;
>> + QSIMPLEQ_INIT(&s->vdisk_aio_retryq);
>> +
>> + /*
>> + * Create a pipe for communicating between two threads in different
>> + * context. Set handler for read event, which gets triggered when
>> + * IO completion is done by non-QEMU context.
>> + */
>> + ret = qemu_pipe(s->fds);
>> + if (ret < 0) {
>> + trace_vxhs_open_epipe('.');
>> + ret = -errno;
>> + goto errout;
>
> This leaks s->vdisk_guid, s->vdisk_hostinfo[i].hostip, etc.
> bdrv_close() will not be called so this function must do cleanup itself.
>
>> + }
>> + fcntl(s->fds[VDISK_FD_READ], F_SETFL, O_NONBLOCK);
>> +
>> + aio_context = bdrv_get_aio_context(bs);
>> + aio_set_fd_handler(aio_context, s->fds[VDISK_FD_READ],
>> + false, vxhs_aio_event_reader, NULL, s);
>> +
>> + /*
>> + * Initialize the spin-locks.
>> + */
>> + qemu_spin_init(&s->vdisk_lock);
>> + qemu_spin_init(&s->vdisk_acb_lock);
>> +
>> + return 0;
>> +
>> +errout:
>> + /*
>> + * Close remote vDisk device if it was opened earlier
>> + */
>> + if (device_opened) {
>
> This is always true. The device_opened variable can be removed.
>
>> +/*
>> + * This allocates QEMU-VXHS callback for each IO
>> + * and is passed to QNIO. When QNIO completes the work,
>> + * it will be passed back through the callback.
>> + */
>> +static BlockAIOCB *vxhs_aio_rw(BlockDriverState *bs,
>> + int64_t sector_num, QEMUIOVector *qiov,
>> + int nb_sectors,
>> + BlockCompletionFunc *cb,
>> + void *opaque, int iodir)
>> +{
>> + VXHSAIOCB *acb = NULL;
>> + BDRVVXHSState *s = bs->opaque;
>> + size_t size;
>> + uint64_t offset;
>> + int iio_flags = 0;
>> + int ret = 0;
>> + void *qnio_ctx = s->qnio_ctx;
>> + uint32_t rfd = s->vdisk_hostinfo[s->vdisk_cur_host_idx].vdisk_rfd;
>> +
>> + offset = sector_num * BDRV_SECTOR_SIZE;
>> + size = nb_sectors * BDRV_SECTOR_SIZE;
>> +
>> + acb = qemu_aio_get(&vxhs_aiocb_info, bs, cb, opaque);
>> + /*
>> + * Setup or initialize VXHSAIOCB.
>> + * Every single field should be initialized since
>> + * acb will be picked up from the slab without
>> + * initializing with zero.
>> + */
>> + acb->io_offset = offset;
>> + acb->size = size;
>> + acb->ret = 0;
>> + acb->flags = 0;
>> + acb->aio_done = VXHS_IO_INPROGRESS;
>> + acb->segments = 0;
>> + acb->buffer = 0;
>> + acb->qiov = qiov;
>> + acb->direction = iodir;
>> +
>> + qemu_spin_lock(&s->vdisk_lock);
>> + if (OF_VDISK_FAILED(s)) {
>> + trace_vxhs_aio_rw(s->vdisk_guid, iodir, size, offset);
>> + qemu_spin_unlock(&s->vdisk_lock);
>> + goto errout;
>> + }
>> + if (OF_VDISK_IOFAILOVER_IN_PROGRESS(s)) {
>> + QSIMPLEQ_INSERT_TAIL(&s->vdisk_aio_retryq, acb, retry_entry);
>> + s->vdisk_aio_retry_qd++;
>> + OF_AIOCB_FLAGS_SET_QUEUED(acb);
>> + qemu_spin_unlock(&s->vdisk_lock);
>> + trace_vxhs_aio_rw_retry(s->vdisk_guid, acb, 1);
>> + goto out;
>> + }
>> + s->vdisk_aio_count++;
>> + qemu_spin_unlock(&s->vdisk_lock);
>> +
>> + iio_flags = (IIO_FLAG_DONE | IIO_FLAG_ASYNC);
>> +
>> + switch (iodir) {
>> + case VDISK_AIO_WRITE:
>> + vxhs_inc_acb_segment_count(acb, 1);
>> + ret = vxhs_qnio_iio_writev(qnio_ctx, rfd, qiov,
>> + offset, (void *)acb, iio_flags);
>> + break;
>> + case VDISK_AIO_READ:
>> + vxhs_inc_acb_segment_count(acb, 1);
>> + ret = vxhs_qnio_iio_readv(qnio_ctx, rfd, qiov,
>> + offset, (void *)acb, iio_flags);
>> + break;
>> + default:
>> + trace_vxhs_aio_rw_invalid(iodir);
>> + goto errout;
>
> s->vdisk_aio_count must be decremented before returning.
>
>> +static void vxhs_close(BlockDriverState *bs)
>> +{
>> + BDRVVXHSState *s = bs->opaque;
>> + int i;
>> +
>> + trace_vxhs_close(s->vdisk_guid);
>> + close(s->fds[VDISK_FD_READ]);
>> + close(s->fds[VDISK_FD_WRITE]);
>> +
>> + /*
>> + * Clearing all the event handlers for oflame registered to QEMU
>> + */
>> + aio_set_fd_handler(bdrv_get_aio_context(bs), s->fds[VDISK_FD_READ],
>> + false, NULL, NULL, NULL);
>
> Please remove the event handler before closing the fd. I don't think it
> matters in this case but in other scenarios there could be race
> conditions if another thread opens an fd and the file descriptor number
> is reused.
^ permalink raw reply
* Re: A VM with 6 volumes - hangs
From: M Ranga Swami Reddy @ 2016-11-14 15:04 UTC (permalink / raw)
To: German Anders; +Cc: ceph-users, ceph-devel
In-Reply-To: <CALKA-0Jr67ZVnoo4zn6sH65oA36xt30_TcQz52dDELAnQr6M9g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1142 bytes --]
When this issue seen, ceph logs shows "slow requests to OSD"
But Ceph status is in OK state.
Thanks
Swami
On Mon, Nov 14, 2016 at 8:27 PM, German Anders <ganders-LWDKa8F8PCdWk0Htik3J/w@public.gmane.org> wrote:
> Could you share some info about the ceph cluster? logs? did you see
> anything different from normal op on the logs?
>
> Best,
>
>
> *German*
>
> 2016-11-14 11:46 GMT-03:00 M Ranga Swami Reddy <swamireddy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>
>> +ceph-devel
>>
>> On Fri, Nov 11, 2016 at 5:09 PM, M Ranga Swami Reddy <
>> swamireddy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> Hello,
>>> I am using the ceph volumes with a VM. Details are below:
>>>
>>> VM:
>>> OS: Ubuntu 14.0.4
>>> CPU: 12 Cores
>>> RAM: 40 GB
>>>
>>> Volumes:
>>> Size: 1 TB
>>> No: 6 Volumes
>>>
>>>
>>> With above, VM got hung without any read/write operation.
>>>
>>> Any suggestions..
>>>
>>> Thanks
>>> Swami
>>>
>>
>>
>> _______________________________________________
>> ceph-users mailing list
>> ceph-users-idqoXFIVOFJgJs9I8MT0rw@public.gmane.org
>> http://lists.ceph.com/listinfo.cgi/ceph-users-ceph.com
>>
>>
>
[-- Attachment #1.2: Type: text/html, Size: 3634 bytes --]
[-- Attachment #2: Type: text/plain, Size: 178 bytes --]
_______________________________________________
ceph-users mailing list
ceph-users-idqoXFIVOFJgJs9I8MT0rw@public.gmane.org
http://lists.ceph.com/listinfo.cgi/ceph-users-ceph.com
^ permalink raw reply
* Re: [GIT PULL] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
From: Алексей Бродкин @ 2016-11-14 15:04 UTC (permalink / raw)
To: Dave Airlie
Cc: airlied, Daniel Vetter, dri-devel, linux-kernel, architt,
Ramiro.Oliveira, Eugeniy.Paltsev, airlied, linux-snps-arc,
Alexey Brodkin
In-Reply-To: <CAML3pwWo0ZFVRNb+JQhpqbDHLpBMj_H-qsLGtDLe_SmfFtOc+w@mail.gmail.com>
Hi Dave,
2016-11-11 4:52 GMT+03:00 Алексей Бродкин <alexey.brodkin@gmail.com>:
> Hi Dave,
>
> Please pull that change for ARC PGU that fixes driver instantiation on
> AXS 10x boards.
> The patch was published for review here:
> https://lists.freedesktop.org/archives/dri-devel/2016-October/121245.html
>
> It is based on today's "drm-next" branch.
I erroneously used "drm-next" instead of "drm-fixes" branch.
I may resend a pull request if you want but that only patch in question gets
applied to "drm-fixes" perfectly fine without any changes.
Regards,
Alexey
^ permalink raw reply
* [GIT PULL] drm/arcpgu: Accommodate adv7511 switch to DRM bridge
From: Алексей Бродкин @ 2016-11-14 15:04 UTC (permalink / raw)
To: linux-snps-arc
In-Reply-To: <CAML3pwWo0ZFVRNb+JQhpqbDHLpBMj_H-qsLGtDLe_SmfFtOc+w@mail.gmail.com>
Hi Dave,
2016-11-11 4:52 GMT+03:00 ??????? ??????? <alexey.brodkin at gmail.com>:
> Hi Dave,
>
> Please pull that change for ARC PGU that fixes driver instantiation on
> AXS 10x boards.
> The patch was published for review here:
> https://lists.freedesktop.org/archives/dri-devel/2016-October/121245.html
>
> It is based on today's "drm-next" branch.
I erroneously used "drm-next" instead of "drm-fixes" branch.
I may resend a pull request if you want but that only patch in question gets
applied to "drm-fixes" perfectly fine without any changes.
Regards,
Alexey
^ permalink raw reply
* Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
From: Christopher Larson @ 2016-11-14 15:03 UTC (permalink / raw)
To: Robert Yang; +Cc: Patches and discussions about the oe-core layer
In-Reply-To: <8c8065fec7073acfbe7c9ea308137e7bb8b83321.1479133967.git.liezhi.yang@windriver.com>
[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]
On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang <liezhi.yang@windriver.com>
wrote:
> The old code:
> network_enabled = not d.getVar('BB_NO_NETWORK', True)
>
> It is True only when BB_NO_NETWORK is not set (None),
> but BB_NO_NETWORK = "0" should also be True while "1" means no network,
> "0" means need network in a normal case.
>
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
> meta/classes/sanity.bbclass | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 7e383f9..c5e3809 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -363,15 +363,19 @@ def check_connectivity(d):
> test_uris = (d.getVar('CONNECTIVITY_CHECK_URIS', True) or "").split()
> retval = ""
>
> + bbn = d.getVar('BB_NO_NETWORK', True)
> + if bbn not in (None, '0', '1'):
> + return 'BB_NO_NETWORK should be "0" or "1", but it is "%s"' % bbn
>
Does this mirror the same logic used in bitbake? What’s the behavior if
it’s set, but to the empty string?
--
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics
[-- Attachment #2: Type: text/html, Size: 1965 bytes --]
^ permalink raw reply
* dm-cache issue
From: Alexander Pashaliyski @ 2016-11-14 15:02 UTC (permalink / raw)
To: dm-devel
[-- Attachment #1.1: Type: text/plain, Size: 663 bytes --]
Hi guys,
I am in a process of evaluating dm-cache for our backup system.
Currently I have an issue when restart the backup server. The server is
booting for hours, because of IO load. It seems is triggered a flush from
SSD disk (that is used for a cache device) to the raid controllers (they
are with slow SATA disks).
I have 10 cached logical volumes in *writethrough mode*, each with 2T of
data over 2 raid controllers. I use a single SSD disk for the cache.
The backup system is with lvm2-2.02.164-1 & kernel 4.4.30.
Do you have any ideas why such flush is triggered? In writethrough cache
mode we shouldn't have dirty blocks in the cache.
Thanks,
Alex
[-- Attachment #1.2: Type: text/html, Size: 772 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH] x86: re-add stack alignment check
From: Jan Beulich @ 2016-11-14 15:02 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Wei Liu
In-Reply-To: <317703fe-ed57-b738-f352-6948281b7c39@citrix.com>
>>> On 14.11.16 at 15:38, <andrew.cooper3@citrix.com> wrote:
> On 14/11/16 14:24, Jan Beulich wrote:
>>>>> On 14.11.16 at 14:45, <andrew.cooper3@citrix.com> wrote:
>>> On 14/11/16 13:25, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/cpu/common.c
>>>> +++ b/xen/arch/x86/cpu/common.c
>>>> @@ -643,6 +643,11 @@ void load_system_tables(void)
>>>> .limit = (IDT_ENTRIES * sizeof(idt_entry_t)) - 1,
>>>> };
>>>>
>>>> + /* Bottom-of-stack must be 16-byte aligned! */
>>>> + BUILD_BUG_ON((sizeof(struct cpu_info) -
>>>> + offsetof(struct cpu_info, guest_cpu_user_regs.es)) & 0xf);
>>>> + BUG_ON(system_state != SYS_STATE_early_boot && (stack_bottom & 0xf));
>>> This will still triple fault the system if it triggers on an AP.
>>> Exceptions aren't hooked up yet.
>> Hmm, true. They could be moved to the very end of the function
>> though?
>
> That would avoid the triple fault, but doesn't make the BUG_ON() useful.
And that's because? (I'm sorry if I'm overlooking the obvious.)
>>> The reason I dropped the check was that there was no way it was going to
>>> fail on the BSP (because of the alignment of cpu0_stack) or APs (because
>>> of the alloc_xenheap_pages(STACK_ORDER, memflags) allocation).
>> These being page aligned has nothing to do with the BUG_ON()
>> triggering. I found its dropping being questionable in the context of
>> the old discussion rooted at this patch of mine
>> https://lists.xenproject.org/archives/html/xen-devel/2010-07/msg00642.html
>> I'd like to particularly point you to the various dependencies which
>> weren't properly spelled out or enforced back then. Specifically
>> it (not) triggering depends on the number and type of fields in
>> struct cpu_info following the guest_cpu_user_regs field.
>
> get_stack_bottom() takes the stack pointer, or's it up to the 8-page
> boundary, overlays a struct cpu_info, then returns the address of es.
>
> There is no value of %rsp which will ever cause it to fail. The only
> think which will cause a failure is the layout of struct cpu_info, but
> the BUILD_BUG_ON() will catch that.
Yes. Otherwise a BUILD_BUG_ON() wouldn't work in the first place.
But please also take into consideration my other reply: Moving the
BUILD_BUG_ON() into get_stack_bottom() is inappropriate, and
having it here without the BUG_ON() risks someone updating code
elsewhere such that the BUILD_BUG_ON() wouldn't trigger, but the
BUG_ON() would. That could be avoided only if we could make the
expression handed to BUILD_BUG_ON() use get_stack_bottom().
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply
* Re: [PATCH 1/3] idle: add support for tasks that inject idle
From: Peter Zijlstra @ 2016-11-14 15:01 UTC (permalink / raw)
To: Jacob Pan
Cc: LKML, Linux PM, Thomas Gleixner, Ingo Molnar, Zhang Rui,
Rafael Wysocki, Chen, Yu C, Sebastian Andrzej Siewior,
Petr Mladek, Srinivas Pandruvada, Arjan van de Ven
In-Reply-To: <20161114145721.GE3142@twins.programming.kicks-ass.net>
On Mon, Nov 14, 2016 at 03:57:21PM +0100, Peter Zijlstra wrote:
> On Wed, Nov 09, 2016 at 11:05:10AM -0800, Jacob Pan wrote:
> > static inline bool is_idle_task(const struct task_struct *p)
> > {
> > - return p->pid == 0;
> > + return !!(p->flags & PF_IDLE);
> > }
>
> > +void play_idle()
> > +{
> > + /*
> > + * Only FIFO tasks can disable the tick since they don't need the forced
> > + * preemption.
> > + */
> > + WARN_ON_ONCE(current->policy != SCHED_FIFO);
> > + WARN_ON_ONCE(current->nr_cpus_allowed != 1);
> > + WARN_ON_ONCE(!(current->flags & PF_KTHREAD));
> > + WARN_ON_ONCE(!(current->flags & PF_NO_SETAFFINITY));
> > + rcu_sleep_check();
> > +
> > + preempt_disable();
> > + current->flags |= PF_IDLE;
> > + do_idle();
> > + current->flags &= ~PF_IDLE;
>
> Whoops, that will clear PF_IDLE from the idle thread. Maybe do something
> like:
>
> unsigned int idle_flag = (~current->flags & PF_IDLE)
>
> current->flags |= PF_IDLE;
>
> current->flags ^= idle_flag;
>
> > +
> > + preempt_fold_need_resched();
> > + preempt_enable();
> > +}
> > +EXPORT_SYMBOL_GPL(play_idle);
n/m I cannot read, the actual idle loop below uses do_idle(), not
play_idle.
> > @@ -299,5 +313,7 @@ void cpu_startup_entry(enum cpuhp_state state)
> > #endif
> > arch_cpu_idle_prepare();
> > cpuhp_online_idle(state);
> > - cpu_idle_loop();
> > + while (1)
> > + do_idle();
> > +
> > }
^ permalink raw reply
* Re: [PATCH v2 1/9] arm64: dts: rockchip: add eMMC's power domain support for rk3399
From: Caesar Wang @ 2016-11-14 15:01 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Caesar Wang, Mark Rutland, devicetree, Elaine Zhang,
Catalin Marinas, Brian Norris, Ziyuan Xu, Will Deacon,
Douglas Anderson, Rob Herring, tfiga,
open list:ARM/Rockchip SoC..., eddie.cai, linux-arm-kernel,
David Wu, Jianqun Xu, linux-kernel
In-Reply-To: <2342292.tK2jqRKrXH@phil>
On 2016年11月14日 22:45, Heiko Stuebner wrote:
> Am Mittwoch, 9. November 2016, 21:21:53 CET schrieb Caesar Wang:
>> From: Ziyuan Xu <xzy.xu@rock-chips.com>
>>
>> Control power domain for eMMC via genpd to reduce power consumption.
>>
>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Authorship / Signed-off mismatch. From above suggest Ziyuan is the author
> while first Signed-off-by indicates Elaine as author. Please clarify.
I believe the Elaine is the first author. Sorry for this kind of
question to brother you again. :(
-
Caesar
>
> Thanks
> Heiko
>
>> ---
>>
>> Changes in v2:
>> - Reviewed-on: https://chromium-review.googlesource.com/376558
>> - Verified on ChromeOS kernel4.4
>>
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index cbb7f8b..b401176 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -269,6 +269,7 @@
>> #clock-cells = <0>;
>> phys = <&emmc_phy>;
>> phy-names = "phy_arasan";
>> + power-domains = <&power RK3399_PD_EMMC>;
>> status = "disabled";
>> };
>>
>> @@ -690,6 +691,11 @@
>> status = "disabled";
>> };
>>
>> + qos_emmc: qos@ffa58000 {
>> + compatible = "syscon";
>> + reg = <0x0 0xffa58000 0x0 0x20>;
>> + };
>> +
>> qos_gmac: qos@ffa5c000 {
>> compatible = "syscon";
>> reg = <0x0 0xffa5c000 0x0 0x20>;
>> @@ -823,6 +829,11 @@
>> };
>>
>> /* These power domains are grouped by VD_LOGIC */
>> + pd_emmc@RK3399_PD_EMMC {
>> + reg = <RK3399_PD_EMMC>;
>> + clocks = <&cru ACLK_EMMC>;
>> + pm_qos = <&qos_emmc>;
>> + };
>> pd_gmac@RK3399_PD_GMAC {
>> reg = <RK3399_PD_GMAC>;
>> clocks = <&cru ACLK_GMAC>;
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
--
caesar wang | software engineer | wxt@rock-chip.com
^ permalink raw reply
* Re: [PATCH v2 02/11] acpi: Define ACPI IO registers for PVH guests
From: Boris Ostrovsky @ 2016-11-14 15:01 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: wei.liu2, ian.jackson, Julien Grall, Paul Durrant, jbeulich,
roger.pau
In-Reply-To: <3d336927-229b-368c-b835-c6850886634a@oracle.com>
On 11/09/2016 04:01 PM, Boris Ostrovsky wrote:
> On 11/09/2016 02:58 PM, Andrew Cooper wrote:
>> On 09/11/16 15:14, Boris Ostrovsky wrote:
>>> On 11/09/2016 09:59 AM, Andrew Cooper wrote:
>>>>> diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
>>>>> index 2e5809b..e3fa704 100644
>>>>> --- a/xen/include/public/hvm/ioreq.h
>>>>> +++ b/xen/include/public/hvm/ioreq.h
>>>>> @@ -24,6 +24,8 @@
>>>>> #ifndef _IOREQ_H_
>>>>> #define _IOREQ_H_
>>>>>
>>>>> +#include "hvm_info_table.h" /* HVM_MAX_VCPUS */
>>>>> +
>>>>> #define IOREQ_READ 1
>>>>> #define IOREQ_WRITE 0
>>>>>
>>>>> @@ -124,6 +126,17 @@ typedef struct buffered_iopage buffered_iopage_t;
>>>>> #define ACPI_GPE0_BLK_ADDRESS ACPI_GPE0_BLK_ADDRESS_V0
>>>>> #define ACPI_GPE0_BLK_LEN ACPI_GPE0_BLK_LEN_V0
>>>>>
>>>>> +#define ACPI_PM1A_EVT_BLK_LEN 0x04
>>>>> +#define ACPI_PM1A_CNT_BLK_LEN 0x02
>>>>> +#define ACPI_PM_TMR_BLK_LEN 0x04
>>>>> +
>>>>> +/* Location of online VCPU bitmap. */
>>>>> +#define ACPI_CPU_MAP 0xaf00
>>>>> +#define ACPI_CPU_MAP_LEN ((HVM_MAX_VCPUS / 8) + \
>>>>> + ((HVM_MAX_VCPUS & 7) ? 1 : 0))
>>>>> +#if ACPI_CPU_MAP + ACPI_CPU_MAP_LEN >= ACPI_GPE0_BLK_ADDRESS_V1
>>>>> +#error "ACPI_CPU_MAP is too big"
>>>>> +#endif
>>>> Why is this in ioreq.h? It has nothing to do with ioreq's.
>>>>
>>>> The current ACPI bits in here are to do with the qemu ACPI interface,
>>>> not the Xen ACPI interface.
>>>>
>>>> Also, please can we avoid hard-coding the location of the map in the
>>>> hypervisor ABI. These constants make it impossible to ever extend the
>>>> number of HVM vcpus at a future date.
>>> The first three logically belong here because corresponding blocks'
>>> addresses are defined right above.
>> They have no relationship to the ones above, other than their name.
> They describe the same object --- for example
> ACPI_PM1A_CNT_BLK_ADDRESS_V1 and (new) ACPI_PM1A_CNT_BLK_LEN describe
> pm1a control.
>
> As far as definitions being there for qemu interface only ---
> ACPI_PM1A_CNT_BLK_ADDRESS_V1, for example, is used only by hvmloader and
> libacpi.
>
>
>>> ACPI_CPU_MAP has to be seen by both the toolstack (libacpi) and the
>>> hypervisor (and qemu as well, although it is defined as
>>> PIIX4_CPU_HOTPLUG_IO_BASE).
>>>
>>> Where do you think it should go then?
>> This highlights a reoccurring problem in Xen which desperately needs
>> fixing, but still isn't high enough on my TODO list to tackle yet.
>>
>> There is no central registration of claims on domain resources. This is
>> the root cause of memory accounting problems for HVM guests.
>>
>>
>> The way I planned to fix this was to have Xen maintain a registry of
>> domains physical resources which ends up looking very much like
>> /proc/io{mem,ports}. There will be a hypercall interface for querying
>> this information, and for a toolstack and device model to modify it.
>>
>> The key point is that Xen needs to be authoritative source of
>> information pertaining to layout, rather than the current fiasco we have
>> of the toolstack, qemu and hvmloader all thinking they know and control
>> what's going on. This fixes several current unknowns which have caused
>> real problems, such as whether a domain was told about certain RMRRs
>> when it booted, or how many PXEROMs qemu tried to fit into the physmap.
>>
>> This information (eventually, when I get Xen-level migration v2 sorted)
>> needs to move at the head of the migration stream.
>>
>> The way I would envisage this working is that on domain create, Xen
>> makes a blank map indicating that all space is free. By selecting
>> X86_EMUL_APCI_*, Xen takes out an allocation when it wires up the ioport
>> handler.
>>
>> Later, when constructing the ACPI tables, the toolstack reads the
>> current ioport allocations and can see exactly which ports are reserved
>> for what.
>>
>>
>> Now, I understand that lumbering you with this work as a prerequisite
>> would be unfair.
>>
>> Therefore, I will accept an alternative of hiding all these definitions
>> behind __XEN_TOOLS__ so the longterm fix can be introduced in a
>> compatible manner in the future.
>
> __XEN_TOOLS__ or (__XEN__ || __XEN_TOOLS__) ? Because both the toolstack
> and the hypervisor want to see them.
>
>
>> That said, I am still certain that they shouldn't live in ioreq.h, as
>> they have nothing to do with Qemu.
> None of the existing files looks (to me) much better in terms of being
> more appropriate. include/public/arch-x86/xen.h?
Andrew, ping on these two questions.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply
* [PATCH v2 1/9] arm64: dts: rockchip: add eMMC's power domain support for rk3399
From: Caesar Wang @ 2016-11-14 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2342292.tK2jqRKrXH@phil>
On 2016?11?14? 22:45, Heiko Stuebner wrote:
> Am Mittwoch, 9. November 2016, 21:21:53 CET schrieb Caesar Wang:
>> From: Ziyuan Xu <xzy.xu@rock-chips.com>
>>
>> Control power domain for eMMC via genpd to reduce power consumption.
>>
>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> Authorship / Signed-off mismatch. From above suggest Ziyuan is the author
> while first Signed-off-by indicates Elaine as author. Please clarify.
I believe the Elaine is the first author. Sorry for this kind of
question to brother you again. :(
-
Caesar
>
> Thanks
> Heiko
>
>> ---
>>
>> Changes in v2:
>> - Reviewed-on: https://chromium-review.googlesource.com/376558
>> - Verified on ChromeOS kernel4.4
>>
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index cbb7f8b..b401176 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -269,6 +269,7 @@
>> #clock-cells = <0>;
>> phys = <&emmc_phy>;
>> phy-names = "phy_arasan";
>> + power-domains = <&power RK3399_PD_EMMC>;
>> status = "disabled";
>> };
>>
>> @@ -690,6 +691,11 @@
>> status = "disabled";
>> };
>>
>> + qos_emmc: qos at ffa58000 {
>> + compatible = "syscon";
>> + reg = <0x0 0xffa58000 0x0 0x20>;
>> + };
>> +
>> qos_gmac: qos at ffa5c000 {
>> compatible = "syscon";
>> reg = <0x0 0xffa5c000 0x0 0x20>;
>> @@ -823,6 +829,11 @@
>> };
>>
>> /* These power domains are grouped by VD_LOGIC */
>> + pd_emmc at RK3399_PD_EMMC {
>> + reg = <RK3399_PD_EMMC>;
>> + clocks = <&cru ACLK_EMMC>;
>> + pm_qos = <&qos_emmc>;
>> + };
>> pd_gmac at RK3399_PD_GMAC {
>> reg = <RK3399_PD_GMAC>;
>> clocks = <&cru ACLK_GMAC>;
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
--
caesar wang | software engineer | wxt at rock-chip.com
^ permalink raw reply
* Re: [PATCH v2 1/9] arm64: dts: rockchip: add eMMC's power domain support for rk3399
From: Caesar Wang @ 2016-11-14 15:01 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Caesar Wang, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
Elaine Zhang, Catalin Marinas, Brian Norris, Ziyuan Xu,
Will Deacon, Douglas Anderson, Rob Herring,
tfiga-F7+t8E8rja9g9hUCZPvPmw, open list:ARM/Rockchip SoC...,
eddie.cai-TNX95d0MmH7DzftRWevZcw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Wu,
Jianqun Xu, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2342292.tK2jqRKrXH@phil>
On 2016年11月14日 22:45, Heiko Stuebner wrote:
> Am Mittwoch, 9. November 2016, 21:21:53 CET schrieb Caesar Wang:
>> From: Ziyuan Xu <xzy.xu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>
>> Control power domain for eMMC via genpd to reduce power consumption.
>>
>> Signed-off-by: Elaine Zhang <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> Signed-off-by: Ziyuan Xu <xzy.xu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Authorship / Signed-off mismatch. From above suggest Ziyuan is the author
> while first Signed-off-by indicates Elaine as author. Please clarify.
I believe the Elaine is the first author. Sorry for this kind of
question to brother you again. :(
-
Caesar
>
> Thanks
> Heiko
>
>> ---
>>
>> Changes in v2:
>> - Reviewed-on: https://chromium-review.googlesource.com/376558
>> - Verified on ChromeOS kernel4.4
>>
>> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index cbb7f8b..b401176 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -269,6 +269,7 @@
>> #clock-cells = <0>;
>> phys = <&emmc_phy>;
>> phy-names = "phy_arasan";
>> + power-domains = <&power RK3399_PD_EMMC>;
>> status = "disabled";
>> };
>>
>> @@ -690,6 +691,11 @@
>> status = "disabled";
>> };
>>
>> + qos_emmc: qos@ffa58000 {
>> + compatible = "syscon";
>> + reg = <0x0 0xffa58000 0x0 0x20>;
>> + };
>> +
>> qos_gmac: qos@ffa5c000 {
>> compatible = "syscon";
>> reg = <0x0 0xffa5c000 0x0 0x20>;
>> @@ -823,6 +829,11 @@
>> };
>>
>> /* These power domains are grouped by VD_LOGIC */
>> + pd_emmc@RK3399_PD_EMMC {
>> + reg = <RK3399_PD_EMMC>;
>> + clocks = <&cru ACLK_EMMC>;
>> + pm_qos = <&qos_emmc>;
>> + };
>> pd_gmac@RK3399_PD_GMAC {
>> reg = <RK3399_PD_GMAC>;
>> clocks = <&cru ACLK_GMAC>;
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
--
caesar wang | software engineer | wxt-TNX95d0MmH73oGB3hsPCZA@public.gmane.org
--
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: [PATCH 1/3] idle: add support for tasks that inject idle
From: Peter Zijlstra @ 2016-11-14 15:01 UTC (permalink / raw)
To: Jacob Pan
Cc: LKML, Linux PM, Thomas Gleixner, Ingo Molnar, Zhang Rui,
Rafael Wysocki, Chen, Yu C, Sebastian Andrzej Siewior,
Petr Mladek, Srinivas Pandruvada, Arjan van de Ven
In-Reply-To: <1478718312-12847-2-git-send-email-jacob.jun.pan@linux.intel.com>
On Wed, Nov 09, 2016 at 11:05:10AM -0800, Jacob Pan wrote:
> +void play_idle()
> +{
> + /*
> + * Only FIFO tasks can disable the tick since they don't need the forced
> + * preemption.
> + */
> + WARN_ON_ONCE(current->policy != SCHED_FIFO);
> + WARN_ON_ONCE(current->nr_cpus_allowed != 1);
> + WARN_ON_ONCE(!(current->flags & PF_KTHREAD));
> + WARN_ON_ONCE(!(current->flags & PF_NO_SETAFFINITY));
> + rcu_sleep_check();
> +
> + preempt_disable();
> + current->flags |= PF_IDLE;
> + do_idle();
> + current->flags &= ~PF_IDLE;
> +
> + preempt_fold_need_resched();
> + preempt_enable();
> +}
> +EXPORT_SYMBOL_GPL(play_idle);
Hurm.. didn't this initially also include the setup of the timer and
take an timeout argument?
^ permalink raw reply
* Re: stmmac/RTL8211F/Meson GXBB: TX throughput problems
From: Andreas Färber @ 2016-11-14 15:00 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: Giuseppe CAVALLARO, Alexandre Torgue, netdev, André Roth,
Johnson Leung, linux-amlogic, Jerome Brunet
In-Reply-To: <CAFBinCDq0neyidQBO13F4Yz-GZPorGyRQ2t5Vb-PL0Ls582+4Q@mail.gmail.com>
Hi,
Am 07.11.2016 um 18:37 schrieb Martin Blumenstingl:
> The same problem still appears on my Tronsmart Vega S95 Meta even with
> the patched PHY driver.
>
> Unfortunately I don't have a second device to rule out that my
> Tronsmart Vega S95 Meta could be broken (not unlikely, I get DDR
> errors from time to time in u-boot). Maybe Andreas Faerber can test
> ethernet with and without Jerome's patch on one of his Tronsmart
> devices.
So far I only ran into the stall issue on my Odroid-C2, not on my Vega
S95 Telos, both on v4.10/integ branch with a rebased MMC patch of yours
on top.
I'm assuming you mean realtek8211f-disable-eee-1000.patch from Nov 3.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* stmmac/RTL8211F/Meson GXBB: TX throughput problems
From: Andreas Färber @ 2016-11-14 15:00 UTC (permalink / raw)
To: linus-amlogic
In-Reply-To: <CAFBinCDq0neyidQBO13F4Yz-GZPorGyRQ2t5Vb-PL0Ls582+4Q@mail.gmail.com>
Hi,
Am 07.11.2016 um 18:37 schrieb Martin Blumenstingl:
> The same problem still appears on my Tronsmart Vega S95 Meta even with
> the patched PHY driver.
>
> Unfortunately I don't have a second device to rule out that my
> Tronsmart Vega S95 Meta could be broken (not unlikely, I get DDR
> errors from time to time in u-boot). Maybe Andreas Faerber can test
> ethernet with and without Jerome's patch on one of his Tronsmart
> devices.
So far I only ran into the stall issue on my Odroid-C2, not on my Vega
S95 Telos, both on v4.10/integ branch with a rebased MMC patch of yours
on top.
I'm assuming you mean realtek8211f-disable-eee-1000.patch from Nov 3.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
^ permalink raw reply
* [patch net-next v2 8/8] mlxsw: packet sample: Add packet sample offloading support
From: Jiri Pirko @ 2016-11-14 15:00 UTC (permalink / raw)
To: netdev
Cc: davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
geert+renesas, stephen, xiyou.wangcong, linux, roopa,
john.fastabend, simon.horman
In-Reply-To: <1479135638-3580-1-git-send-email-jiri@resnulli.us>
From: Yotam Gigi <yotamg@mellanox.com>
Using the MPSC regiter, add the functions that configure port packets
sampling in hardware and the necessary datatypes in the mlxsw_sp_port
struct. In addition, add the necessary trap for sampled packets and
integrate with matchall offloading to allow offloading of the sample tc
action.
The current offload support is for the tc command:
tc filter add dev <DEV> parent ffff: \
matchall \
action sample rate <RATE> mark <MARK> [trunc <SIZE>] \
[src <SADDR>] [dst <DADDR>] [type <TYPE>]
Where only ingress qdiscs are supported, and only a combination of
matchall classifier and sample action will lead to activating hardware
packet sampling.
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 120 +++++++++++++++++++++++--
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 12 +++
drivers/net/ethernet/mellanox/mlxsw/trap.h | 1 +
3 files changed, 125 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index a5433e4..6cbf67c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -57,6 +57,8 @@
#include <net/pkt_cls.h>
#include <net/tc_act/tc_mirred.h>
#include <net/netevent.h>
+#include <net/tc_act/tc_sample.h>
+#include <net/ife.h>
#include "spectrum.h"
#include "pci.h"
@@ -467,6 +469,16 @@ static void mlxsw_sp_span_mirror_remove(struct mlxsw_sp_port *from,
mlxsw_sp_span_inspected_port_unbind(from, span_entry, type);
}
+static int mlxsw_sp_port_sample_set(struct mlxsw_sp_port *mlxsw_sp_port,
+ bool enable, u32 rate)
+{
+ struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+ char mpsc_pl[MLXSW_REG_MPSC_LEN];
+
+ mlxsw_reg_mpsc_pack(mpsc_pl, mlxsw_sp_port->local_port, enable, rate);
+ return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(mpsc), mpsc_pl);
+}
+
static int mlxsw_sp_port_admin_status_set(struct mlxsw_sp_port *mlxsw_sp_port,
bool is_up)
{
@@ -1221,6 +1233,49 @@ mlxsw_sp_port_add_cls_matchall_mirror(struct mlxsw_sp_port *mlxsw_sp_port,
return err;
}
+static int
+mlxsw_sp_port_add_cls_matchall_sample(struct mlxsw_sp_port *mlxsw_sp_port,
+ struct tc_cls_matchall_offload *cls,
+ const struct tc_action *a,
+ bool ingress)
+{
+ struct mlxsw_sp_port_mall_tc_entry *mall_tc_entry;
+ int sampler_id;
+ int err;
+
+ if (mlxsw_sp_port->sample.enable) {
+ netdev_err(mlxsw_sp_port->dev, "Sample already active\n");
+ return -EEXIST;
+ }
+
+ err = mlxsw_sp_port_sample_set(mlxsw_sp_port, true, tcf_sample_rate(a));
+ if (err)
+ return err;
+
+ sampler_id = tcf_sample_sampler_id(a);
+
+ mlxsw_sp_port->sample.enable = true;
+ mlxsw_sp_port->sample.mark = tcf_sample_mark(a);
+ mlxsw_sp_port->sample.truncate = tcf_sample_truncate(a);
+ mlxsw_sp_port->sample.trunc_size = tcf_sample_trunc_size(a);
+ mlxsw_sp_port->sample.eth_type = tcf_sample_eth_type(a);
+ mlxsw_sp_port->sample.sampler_id = TC_SAMPLE_HW_ID(sampler_id);
+ tcf_sample_eth_dst_addr(a, mlxsw_sp_port->sample.eth_dst);
+ tcf_sample_eth_src_addr(a, mlxsw_sp_port->sample.eth_src);
+
+ netdev_dbg(mlxsw_sp_port->dev, "Activate hardware sample\n");
+
+ mall_tc_entry = kzalloc(sizeof(*mall_tc_entry), GFP_KERNEL);
+ if (!mall_tc_entry)
+ return -ENOMEM;
+
+ mall_tc_entry->cookie = cls->cookie;
+ mall_tc_entry->type = MLXSW_SP_PORT_MALL_SAMPLE;
+ list_add_tail(&mall_tc_entry->list, &mlxsw_sp_port->mall_tc_list);
+
+ return 0;
+}
+
static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
__be16 protocol,
struct tc_cls_matchall_offload *cls,
@@ -1236,17 +1291,19 @@ static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
}
tcf_exts_to_list(cls->exts, &actions);
- list_for_each_entry(a, &actions, list) {
- if (!is_tcf_mirred_egress_mirror(a) ||
- protocol != htons(ETH_P_ALL)) {
- return -ENOTSUPP;
- }
+ a = list_first_entry(&actions, struct tc_action, list);
+ if (is_tcf_mirred_egress_mirror(a) && protocol == htons(ETH_P_ALL))
err = mlxsw_sp_port_add_cls_matchall_mirror(mlxsw_sp_port, cls,
a, ingress);
- if (err)
- return err;
- }
+ else if (is_tcf_sample(a) && protocol == htons(ETH_P_ALL))
+ err = mlxsw_sp_port_add_cls_matchall_sample(mlxsw_sp_port, cls,
+ a, ingress);
+ else
+ return -ENOTSUPP;
+
+ if (err)
+ return err;
return 0;
}
@@ -1274,6 +1331,10 @@ static void mlxsw_sp_port_del_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
mlxsw_sp_span_mirror_remove(mlxsw_sp_port, to_port, span_type);
break;
+ case MLXSW_SP_PORT_MALL_SAMPLE:
+ mlxsw_sp_port->sample.enable = false;
+ mlxsw_sp_port_sample_set(mlxsw_sp_port, false, 1);
+ break;
default:
WARN_ON(1);
}
@@ -2774,6 +2835,46 @@ static void mlxsw_sp_rx_listener_mark_func(struct sk_buff *skb, u8 local_port,
return mlxsw_sp_rx_listener_func(skb, local_port, priv);
}
+static void mlxsw_sp_rx_listener_sample_func(struct sk_buff *skb, u8 local_port,
+ void *priv)
+{
+ struct mlxsw_sp *mlxsw_sp = priv;
+ struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp->ports[local_port];
+ static struct ethhdr *ethhdr;
+ int orig_size;
+
+ if (unlikely(!mlxsw_sp_port)) {
+ dev_warn_ratelimited(mlxsw_sp->bus_info->dev, "Port %d: sample skb received for non-existent port\n",
+ local_port);
+ return;
+ }
+
+ skb->dev = mlxsw_sp_port->dev;
+ skb->mac_len = skb->dev->hard_header_len;
+ skb->mark = mlxsw_sp_port->sample.mark;
+
+ orig_size = skb->len;
+
+ if (mlxsw_sp_port->sample.truncate)
+ skb_trim(skb, mlxsw_sp_port->sample.trunc_size);
+
+ ethhdr = ife_packet_info_pack(skb, orig_size, skb->dev->ifindex, 0,
+ mlxsw_sp_port->sample.sampler_id,
+ mlxsw_sp_port->sample.seq++);
+ if (!ethhdr)
+ return;
+
+ if (!is_zero_ether_addr(mlxsw_sp_port->sample.eth_src))
+ ether_addr_copy(ethhdr->h_source,
+ mlxsw_sp_port->sample.eth_src);
+ if (!is_zero_ether_addr(mlxsw_sp_port->sample.eth_dst))
+ ether_addr_copy(ethhdr->h_dest, mlxsw_sp_port->sample.eth_dst);
+ ethhdr->h_proto = htons(mlxsw_sp_port->sample.eth_type);
+
+ skb->protocol = eth_type_trans(skb, skb->dev);
+ netif_receive_skb(skb);
+}
+
#define MLXSW_SP_RXL(_func, _trap_id, _action) \
{ \
.func = _func, \
@@ -2784,6 +2885,9 @@ static void mlxsw_sp_rx_listener_mark_func(struct sk_buff *skb, u8 local_port,
static const struct mlxsw_rx_listener mlxsw_sp_rx_listener[] = {
MLXSW_SP_RXL(mlxsw_sp_rx_listener_func, FDB_MC, TRAP_TO_CPU),
+ MLXSW_SP_RXL(mlxsw_sp_rx_listener_sample_func, PKT_SAMPLE,
+ MIRROR_TO_CPU),
+
/* Traps for specific L2 packet types, not trapped as FDB MC */
MLXSW_SP_RXL(mlxsw_sp_rx_listener_func, STP, TRAP_TO_CPU),
MLXSW_SP_RXL(mlxsw_sp_rx_listener_func, LACP, TRAP_TO_CPU),
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 04a2bc7..7f2e6fc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -229,6 +229,7 @@ struct mlxsw_sp_span_entry {
enum mlxsw_sp_port_mall_action_type {
MLXSW_SP_PORT_MALL_MIRROR,
+ MLXSW_SP_PORT_MALL_SAMPLE,
};
struct mlxsw_sp_port_mall_mirror_tc_entry {
@@ -361,6 +362,17 @@ struct mlxsw_sp_port {
struct rtnl_link_stats64 *cache;
struct delayed_work update_dw;
} hw_stats;
+ struct {
+ bool enable;
+ u32 mark;
+ bool truncate;
+ u32 trunc_size;
+ u8 eth_src[ETH_ALEN];
+ u8 eth_dst[ETH_ALEN];
+ u16 eth_type;
+ u32 sampler_id;
+ u32 seq;
+ } sample;
};
struct mlxsw_sp_port *mlxsw_sp_port_lower_dev_hold(struct net_device *dev);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/trap.h b/drivers/net/ethernet/mellanox/mlxsw/trap.h
index ed8e301..eebd135 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/trap.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/trap.h
@@ -54,6 +54,7 @@ enum {
MLXSW_TRAP_ID_IGMP_V2_REPORT = 0x32,
MLXSW_TRAP_ID_IGMP_V2_LEAVE = 0x33,
MLXSW_TRAP_ID_IGMP_V3_REPORT = 0x34,
+ MLXSW_TRAP_ID_PKT_SAMPLE = 0x38,
MLXSW_TRAP_ID_ARPBC = 0x50,
MLXSW_TRAP_ID_ARPUC = 0x51,
MLXSW_TRAP_ID_MTUERROR = 0x52,
--
2.7.4
^ permalink raw reply related
* [patch net-next v2 7/8] mlxsw: reg: add the Monitoring Packet Sampling Configuration Register
From: Jiri Pirko @ 2016-11-14 15:00 UTC (permalink / raw)
To: netdev
Cc: davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
geert+renesas, stephen, xiyou.wangcong, linux, roopa,
john.fastabend, simon.horman
In-Reply-To: <1479135638-3580-1-git-send-email-jiri@resnulli.us>
From: Yotam Gigi <yotamg@mellanox.com>
The MPSC register allows to configure ingress packet sampling on specific
port of the mlxsw device. The sampled packets are then trapped via
PKT_SAMPLE trap.
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 38 +++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index edad7cb..b5ab81a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -4771,6 +4771,44 @@ static inline void mlxsw_reg_mlcr_pack(char *payload, u8 local_port,
MLXSW_REG_MLCR_DURATION_MAX : 0);
}
+/* MPSC - Monitoring Packet Sampling Configuration Register
+ * --------------------------------------------------------
+ * MPSC Register is used to configure the Packet Sampling mechanism.
+ */
+#define MLXSW_REG_MPSC_ID 0x9080
+#define MLXSW_REG_MPSC_LEN 0x14
+
+MLXSW_REG_DEFINE(mpsc, MLXSW_REG_MPSC_ID, MLXSW_REG_MPSC_LEN);
+
+/* reg_mpsc_local_port
+ * Local port number
+ * Not supported for CPU port
+ * Access: Index
+ */
+MLXSW_ITEM32(reg, mpsc, local_port, 0x00, 16, 8);
+
+/* reg_mpsc_e
+ * Enable sampling on port local_port
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mpsc, e, 0x04, 30, 1);
+
+/* reg_mpsc_rate
+ * Sampling rate = 1 out of rate packets (with randomization around
+ * the point). Valid values are: 1 to 3.5*10^9
+ * Access: RW
+ */
+MLXSW_ITEM32(reg, mpsc, rate, 0x08, 0, 32);
+
+static inline void mlxsw_reg_mpsc_pack(char *payload, u8 local_port, bool e,
+ u32 rate)
+{
+ MLXSW_REG_ZERO(mpsc, payload);
+ mlxsw_reg_mpsc_local_port_set(payload, local_port);
+ mlxsw_reg_mpsc_e_set(payload, e);
+ mlxsw_reg_mpsc_rate_set(payload, rate);
+}
+
/* SBPR - Shared Buffer Pools Register
* -----------------------------------
* The SBPR configures and retrieves the shared buffer pools and configuration.
--
2.7.4
^ permalink raw reply related
* [patch net-next v2 6/8] tc: sample: Add sequence number and sampler_id fields
From: Jiri Pirko @ 2016-11-14 15:00 UTC (permalink / raw)
To: netdev
Cc: davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
geert+renesas, stephen, xiyou.wangcong, linux, roopa,
john.fastabend, simon.horman
In-Reply-To: <1479135638-3580-1-git-send-email-jiri@resnulli.us>
From: Yotam Gigi <yotamg@mellanox.com>
Add support for sequence numbering to the sampled packets. The sequence
number counts the sampled packets and enables to check whether sampled
packets have been dropped. The sequence state is kept per sample action
instance.
Add unique id (sampler_id) for each sample action instance, and add that
sampler_id to the packet. This way, a user application can verify sample
drops, by tracking the sequence numbers from each sampler id.
Because of the fact that hw and sw can be triggered by the same tc action
(if the user does not specify both skip_sw and skip_hw), each should
have a different sampler_id, as each generates a different sequence
numbering. To allow that, the macros TC_SAMPLE_HW_ID and TC_SAMPLE_SW_ID,
which, given a sampler_id, calculates the hw sample_id and sw sample_id.
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/ife.h | 5 +++--
include/net/tc_act/tc_sample.h | 9 +++++++++
include/uapi/linux/ife.h | 2 ++
net/ife/ife.c | 14 ++++++++++++--
net/sched/act_sample.c | 10 +++++++++-
5 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/include/net/ife.h b/include/net/ife.h
index 5fbcfb3..3ae4f0d 100644
--- a/include/net/ife.h
+++ b/include/net/ife.h
@@ -19,7 +19,8 @@ int ife_tlv_meta_encode(void *skbdata, u16 attrtype, u16 dlen,
void *ife_tlv_meta_next(void *skbdata);
struct ethhdr *ife_packet_info_pack(struct sk_buff *skb, int orig_size,
- int in_ifindex, int out_ifindex);
+ int in_ifindex, int out_ifindex,
+ u32 sampler_id, u32 seq);
#else
@@ -52,7 +53,7 @@ static inline void *ife_tlv_meta_next(void *skbdata)
static inline struct ethhdr *
ife_packet_info_pack(struct sk_buff *skb, int orig_size, int in_ifindex,
- int out_ifindex)
+ int out_ifindex, u32 sampler_id, u32 seq)
{
return NULL;
}
diff --git a/include/net/tc_act/tc_sample.h b/include/net/tc_act/tc_sample.h
index af99eb8..07638b7 100644
--- a/include/net/tc_act/tc_sample.h
+++ b/include/net/tc_act/tc_sample.h
@@ -13,9 +13,13 @@ struct tcf_sample {
u8 eth_dst[ETH_ALEN];
u8 eth_src[ETH_ALEN];
u16 eth_type;
+ u32 sampler_id;
+ u32 seq;
struct list_head tcfm_list;
};
#define to_sample(a) ((struct tcf_sample *)a)
+#define TC_SAMPLE_SW_ID(id) ((id << 1) + 0)
+#define TC_SAMPLE_HW_ID(id) ((id << 1) + 1)
static inline bool is_tcf_sample(const struct tc_action *a)
{
@@ -61,4 +65,9 @@ static inline void tcf_sample_eth_src_addr(const struct tc_action *a, u8 *src)
ether_addr_copy(src, to_sample(a)->eth_src);
}
+static inline u32 tcf_sample_sampler_id(const struct tc_action *a)
+{
+ return to_sample(a)->sampler_id;
+}
+
#endif /* __NET_TC_SAMPLE_H */
diff --git a/include/uapi/linux/ife.h b/include/uapi/linux/ife.h
index ebdd785..2007818 100644
--- a/include/uapi/linux/ife.h
+++ b/include/uapi/linux/ife.h
@@ -13,6 +13,8 @@ enum {
IFE_META_OUT_IFINDEX,
IFE_META_ORIGSIZE,
IFE_META_SIZE,
+ IFE_META_SAMPLER_ID,
+ IFE_META_SEQ,
__IFE_META_MAX
};
diff --git a/net/ife/ife.c b/net/ife/ife.c
index f0b683e..e1b7452d 100644
--- a/net/ife/ife.c
+++ b/net/ife/ife.c
@@ -139,7 +139,8 @@ int ife_tlv_meta_encode(void *skbdata, u16 attrtype, u16 dlen, const void *dval)
EXPORT_SYMBOL_GPL(ife_tlv_meta_encode);
struct ethhdr *ife_packet_info_pack(struct sk_buff *skb, int orig_size,
- int in_ifindex, int out_ifindex)
+ int in_ifindex, int out_ifindex,
+ u32 sampler_id, u32 seq)
{
int curr_size;
void *ifetlv;
@@ -148,7 +149,9 @@ struct ethhdr *ife_packet_info_pack(struct sk_buff *skb, int orig_size,
curr_size = skb->len;
metalen = nla_total_size(sizeof(orig_size)) +
- nla_total_size(sizeof(curr_size));
+ nla_total_size(sizeof(curr_size)) +
+ nla_total_size(sizeof(sampler_id)) +
+ nla_total_size(sizeof(seq));
if (in_ifindex)
metalen += nla_total_size(sizeof(in_ifindex));
@@ -159,6 +162,8 @@ struct ethhdr *ife_packet_info_pack(struct sk_buff *skb, int orig_size,
out_ifindex = htonl(out_ifindex);
orig_size = htonl(orig_size);
curr_size = htonl(curr_size);
+ sampler_id = htonl(sampler_id);
+ seq = htonl(seq);
ifetlv = ife_encode(skb, metalen);
if (!ifetlv)
@@ -179,6 +184,11 @@ struct ethhdr *ife_packet_info_pack(struct sk_buff *skb, int orig_size,
ifetlv += ife_tlv_meta_encode(ifetlv, IFE_META_SIZE,
sizeof(curr_size), &curr_size);
+ ifetlv += ife_tlv_meta_encode(ifetlv, IFE_META_SAMPLER_ID,
+ sizeof(sampler_id), &sampler_id);
+
+ ifetlv += ife_tlv_meta_encode(ifetlv, IFE_META_SEQ, sizeof(seq), &seq);
+
return (struct ethhdr *) skb->data;
}
EXPORT_SYMBOL(ife_packet_info_pack);
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 39f1d94c..3fe9489 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -27,6 +27,7 @@
#define SAMPLE_TAB_MASK 7
static int sample_net_id;
+static u32 samplers;
static struct tc_action_ops act_sample_ops;
static const struct nla_policy sample_policy[TCA_SAMPLE_MAX + 1] = {
@@ -77,6 +78,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
s->rate = nla_get_u32(tb[TCA_SAMPLE_RATE]);
s->mark = nla_get_u32(tb[TCA_SAMPLE_MARK]);
s->eth_type = nla_get_u16(tb[TCA_SAMPLE_ETH_TYPE]);
+ s->sampler_id = samplers++;
s->truncate = tb[TCA_SAMPLE_TRUNC_SIZE];
if (tb[TCA_SAMPLE_TRUNC_SIZE])
@@ -117,6 +119,7 @@ static int tcf_sample(struct sk_buff *skb, const struct tc_action *a,
struct tcf_sample *s = to_sample(a);
static struct ethhdr *ethhdr;
struct sk_buff *skb2;
+ u32 sampler_id;
int orig_size;
int retval;
u32 at;
@@ -144,8 +147,12 @@ static int tcf_sample(struct sk_buff *skb, const struct tc_action *a,
skb_push(skb2, skb2->mac_len);
orig_size = skb->len + skb->dev->hard_header_len;
+ sampler_id = TC_SAMPLE_SW_ID(s->sampler_id);
+
ethhdr = ife_packet_info_pack(skb2, orig_size,
- skb->dev->ifindex, 0);
+ skb->dev->ifindex, 0,
+ sampler_id, s->seq++);
+
if (!ethhdr)
goto out;
@@ -266,6 +273,7 @@ static struct pernet_operations sample_net_ops = {
static int __init sample_init_module(void)
{
+ samplers = 0;
return tcf_register_action(&act_sample_ops, &sample_net_ops);
}
--
2.7.4
^ permalink raw reply related
* Re: [Qemu-devel] [qemu patch 2/2] kvmclock: reduce kvmclock difference on migration
From: Paolo Bonzini @ 2016-11-14 15:00 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: kvm, qemu-devel, Dr. David Alan Gilbert, Juan Quintela,
Radim Krcmar, Eduardo Habkost
In-Reply-To: <20161114145054.GA28663@amt.cnet>
On 14/11/2016 15:50, Marcelo Tosatti wrote:
> Well, i didnt want to mix the meaning of the variables:
>
> + /* whether machine supports reliable KVM_GET_CLOCK */
> + bool mach_use_reliable_get_clock;
> +
> + /* whether source host supported reliable KVM_GET_CLOCK */
> + bool src_use_reliable_get_clock;
>
> See the comments on top (later if you look at the variable,
> then have to think: well it has one name, but its disabled
> by that other path as well, so its more than its
> name,etc...).
>
>> I'm thinking "mach_use_reliable_get_clock is just for migration,
>
> Thats whether the machine supports it. New machines have it enabled,
> olders don't.
Yes.
>> src_use_reliable_get_clock is the state".
>
> Thats whether the migration source supported it.
But it's not used only for migration. It's used on every vmstate change
(running->stop and stop->running, isn't it? I think that, apart from
the migration case, it's better to use s->clock if kvmclock is stable,
even on older machine types.
>>>>> +static bool kvmclock_src_use_reliable_get_clock(void *opaque)
>>>>> +{
>>>>> + KVMClockState *s = opaque;
>>>>> +
>>>>> + /*
>>>>> + * On machine types that support reliable KVM_GET_CLOCK,
>>>>> + * if host kernel does provide reliable KVM_GET_CLOCK,
>>>>> + * set src_use_reliable_get_clock=true so that destination
>>>>> + * avoids reading kvmclock from memory.
>>>>> + */
>>>>> + if (s->mach_use_reliable_get_clock && kvm_has_adjust_clock_stable()) {
>>>>> + s->src_use_reliable_get_clock = true;
>>>>> + }
>>>>> +
>>>>> + return s->src_use_reliable_get_clock;
>>>>> +}
>>>>
>>>> Here you can just return s->mach_use_reliable_get_clock.
>>>
>>> mach_use_reliable_get_clock can be true but host might not support it.
>>
>> Yes, but the "needed" function is only required to avoid breaking
>> pc-i440fx-2.7 and earlier.
>
> "needed" is required so that the migration between:
>
> SRC DEST BEHAVIOUR
> ~support supports on migration read from guest,
> on stop/cont use
> kvm_get_clock/kvm_set_clock
>
> Destination does not use KVM_GET_CLOCK value (which is
> broken and should not be used).
If needed returns false, the destination will see
src_use_reliable_get_clock = false anyway.
If needed returns true, the destination can still see
src_use_reliable_get_clock = false if that's the value.
needed src_use_reliable_get_clock effect
false false use kvmclock_current_nsec
false true use kvmclock_current_nsec
true false use kvmclock_current_nsec
true true use s->clock
So the idea is:
- set s->clock and s->reliable_get_clock on every KVM_GET_CLOCK
- on migration source, use subsections and
s->mach_use_reliable_get_clock to avoid breaking migration on pre-2.8
machine types
- on migration destination, use .pre_load so that s->reliable_get_clock
is initialized to false on older machine types
>> If you return true here, you can still
>> migrate a "false" value for src_use_reliable_get_clock.
>
> But the source only uses a reliable KVM_GET_CLOCK if
> both conditions are true.
>
> And the subsection is only needed if the source
> uses a reliable KVM_GET_CLOCK.
Yes, but the point of the subsection is just to avoid breaking migration
format.
>> It is the same as "is using masterclock", which is actually a stricter
>> condition than the KVM_CHECK_EXTENSION return value. The right check to
>> use is whether masterclock is in use,
>
> Actually its "has a reliable KVM_GET_CLOCK" (which returns
> get_kernel_clock() + (rdtsc() - tsc_timestamp),
>
> "broken KVM_GET_CLOCK" = get_kernel_clock()
Yes. And these end up being the same.
Paolo
^ permalink raw reply
* [patch net-next v2 5/8] Introduce sample tc action
From: Jiri Pirko @ 2016-11-14 15:00 UTC (permalink / raw)
To: netdev
Cc: davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
geert+renesas, stephen, xiyou.wangcong, linux, roopa,
john.fastabend, simon.horman
In-Reply-To: <1479135638-3580-1-git-send-email-jiri@resnulli.us>
From: Yotam Gigi <yotamg@mellanox.com>
This action allow the user to sample traffic matched by tc classifier.
The sampling consists of choosing packets randomly, truncating them,
adding some informative metadata regarding the interface and the original
packet size and mark them with specific mark, to allow further tc rules to
match and process. The marked sample packets are then injected into the
device ingress qdisc using netif_receive_skb.
The packets metadata is packed using the ife encapsulation protocol, and
the outer packet's ethernet dest, source and eth_type, along with the
rate, mark and the optional truncation size can be configured from
userspace.
Example:
To sample ingress traffic from interface eth1, and redirect the sampled
the sampled packets to interface dummy0, one may use the commands:
tc qdisc add dev eth1 handle ffff: ingress
tc filter add dev eth1 parent ffff: \
matchall action sample rate 12 mark 17
tc filter add parent ffff: dev eth1 protocol all \
u32 match mark 17 0xff \
action mirred egress redirect dev dummy0
Where the first command adds an ingress qdisc and the second starts
sampling randomly with an average of one sampled packet per 12 packets on
dev eth1 and marks the sampled packets with 17. The third command catches
the sampled packets, which are marked with 17, and redirects them to dev
dummy0.
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/tc_act/tc_sample.h | 64 ++++++++
include/uapi/linux/tc_act/Kbuild | 1 +
include/uapi/linux/tc_act/tc_sample.h | 29 ++++
net/sched/Kconfig | 13 ++
net/sched/Makefile | 1 +
net/sched/act_sample.c | 282 ++++++++++++++++++++++++++++++++++
6 files changed, 390 insertions(+)
create mode 100644 include/net/tc_act/tc_sample.h
create mode 100644 include/uapi/linux/tc_act/tc_sample.h
create mode 100644 net/sched/act_sample.c
diff --git a/include/net/tc_act/tc_sample.h b/include/net/tc_act/tc_sample.h
new file mode 100644
index 0000000..af99eb8
--- /dev/null
+++ b/include/net/tc_act/tc_sample.h
@@ -0,0 +1,64 @@
+#ifndef __NET_TC_SAMPLE_H
+#define __NET_TC_SAMPLE_H
+
+#include <net/act_api.h>
+#include <linux/tc_act/tc_sample.h>
+
+struct tcf_sample {
+ struct tc_action common;
+ u32 rate;
+ u32 mark;
+ bool truncate;
+ u32 trunc_size;
+ u8 eth_dst[ETH_ALEN];
+ u8 eth_src[ETH_ALEN];
+ u16 eth_type;
+ struct list_head tcfm_list;
+};
+#define to_sample(a) ((struct tcf_sample *)a)
+
+static inline bool is_tcf_sample(const struct tc_action *a)
+{
+#ifdef CONFIG_NET_CLS_ACT
+ return a->ops && a->ops->type == TCA_ACT_SAMPLE;
+#else
+ return false;
+#endif
+}
+
+static inline __u32 tcf_sample_mark(const struct tc_action *a)
+{
+ return to_sample(a)->mark;
+}
+
+static inline __u32 tcf_sample_rate(const struct tc_action *a)
+{
+ return to_sample(a)->rate;
+}
+
+static inline bool tcf_sample_truncate(const struct tc_action *a)
+{
+ return to_sample(a)->truncate;
+}
+
+static inline int tcf_sample_trunc_size(const struct tc_action *a)
+{
+ return to_sample(a)->trunc_size;
+}
+
+static inline u16 tcf_sample_eth_type(const struct tc_action *a)
+{
+ return to_sample(a)->eth_type;
+}
+
+static inline void tcf_sample_eth_dst_addr(const struct tc_action *a, u8 *dst)
+{
+ ether_addr_copy(dst, to_sample(a)->eth_dst);
+}
+
+static inline void tcf_sample_eth_src_addr(const struct tc_action *a, u8 *src)
+{
+ ether_addr_copy(src, to_sample(a)->eth_src);
+}
+
+#endif /* __NET_TC_SAMPLE_H */
diff --git a/include/uapi/linux/tc_act/Kbuild b/include/uapi/linux/tc_act/Kbuild
index e3969bd..6c6b8d6 100644
--- a/include/uapi/linux/tc_act/Kbuild
+++ b/include/uapi/linux/tc_act/Kbuild
@@ -4,6 +4,7 @@ header-y += tc_defact.h
header-y += tc_gact.h
header-y += tc_ipt.h
header-y += tc_mirred.h
+header-y += tc_sample.h
header-y += tc_nat.h
header-y += tc_pedit.h
header-y += tc_skbedit.h
diff --git a/include/uapi/linux/tc_act/tc_sample.h b/include/uapi/linux/tc_act/tc_sample.h
new file mode 100644
index 0000000..44ee9d0
--- /dev/null
+++ b/include/uapi/linux/tc_act/tc_sample.h
@@ -0,0 +1,29 @@
+#ifndef __LINUX_TC_SAMPLE_H
+#define __LINUX_TC_SAMPLE_H
+
+#include <linux/types.h>
+#include <linux/pkt_cls.h>
+#include <linux/if_ether.h>
+
+#define TCA_ACT_SAMPLE 26
+
+struct tc_sample {
+ tc_gen;
+};
+
+enum {
+ TCA_SAMPLE_UNSPEC,
+ TCA_SAMPLE_PARMS,
+ TCA_SAMPLE_TM,
+ TCA_SAMPLE_RATE,
+ TCA_SAMPLE_MARK,
+ TCA_SAMPLE_TRUNC_SIZE,
+ TCA_SAMPLE_ETH_DST,
+ TCA_SAMPLE_ETH_SRC,
+ TCA_SAMPLE_ETH_TYPE,
+ TCA_SAMPLE_PAD,
+ __TCA_SAMPLE_MAX
+};
+#define TCA_SAMPLE_MAX (__TCA_SAMPLE_MAX - 1)
+
+#endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 24f7cac..c54ea6b 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -650,6 +650,19 @@ config NET_ACT_MIRRED
To compile this code as a module, choose M here: the
module will be called act_mirred.
+config NET_ACT_SAMPLE
+ tristate "Traffic Sampling"
+ depends on NET_CLS_ACT
+ select NET_IFE
+ ---help---
+ Say Y here to allow packet sampling tc action. The packet sample
+ action consists of statistically duplicating packets, truncating them
+ and adding descriptive metadata to them. The duplicated packets are
+ then marked to allow further processing using tc.
+
+ To compile this code as a module, choose M here: the
+ module will be called act_sample.
+
config NET_ACT_IPT
tristate "IPtables targets"
depends on NET_CLS_ACT && NETFILTER && IP_NF_IPTABLES
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 4bdda36..7b915d2 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_NET_CLS_ACT) += act_api.o
obj-$(CONFIG_NET_ACT_POLICE) += act_police.o
obj-$(CONFIG_NET_ACT_GACT) += act_gact.o
obj-$(CONFIG_NET_ACT_MIRRED) += act_mirred.o
+obj-$(CONFIG_NET_ACT_SAMPLE) += act_sample.o
obj-$(CONFIG_NET_ACT_IPT) += act_ipt.o
obj-$(CONFIG_NET_ACT_NAT) += act_nat.o
obj-$(CONFIG_NET_ACT_PEDIT) += act_pedit.o
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
new file mode 100644
index 0000000..39f1d94c
--- /dev/null
+++ b/net/sched/act_sample.c
@@ -0,0 +1,282 @@
+/*
+ * net/sched/act_sample.c - Packet samplig tc action
+ * Copyright (c) 2016 Yotam Gigi <yotamg@mellanox.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <linux/rtnetlink.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/gfp.h>
+#include <net/net_namespace.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+#include <linux/tc_act/tc_sample.h>
+#include <net/tc_act/tc_sample.h>
+#include <net/ife.h>
+
+#include <linux/if_arp.h>
+
+#define SAMPLE_TAB_MASK 7
+static int sample_net_id;
+static struct tc_action_ops act_sample_ops;
+
+static const struct nla_policy sample_policy[TCA_SAMPLE_MAX + 1] = {
+ [TCA_SAMPLE_PARMS] = { .len = sizeof(struct tc_sample) },
+};
+
+static int tcf_sample_init(struct net *net, struct nlattr *nla,
+ struct nlattr *est, struct tc_action **a, int ovr,
+ int bind)
+{
+ struct tc_action_net *tn = net_generic(net, sample_net_id);
+ struct nlattr *tb[TCA_SAMPLE_MAX + 1];
+ struct tc_sample *parm;
+ struct tcf_sample *s;
+ int ret;
+ bool exists = false;
+
+ if (!nla)
+ return -EINVAL;
+ ret = nla_parse_nested(tb, TCA_SAMPLE_MAX, nla, sample_policy);
+ if (ret < 0)
+ return ret;
+ if (!tb[TCA_SAMPLE_PARMS] || !tb[TCA_SAMPLE_RATE] ||
+ !tb[TCA_SAMPLE_MARK] || !tb[TCA_SAMPLE_ETH_TYPE])
+ return -EINVAL;
+
+ parm = nla_data(tb[TCA_SAMPLE_PARMS]);
+
+ exists = tcf_hash_check(tn, parm->index, a, bind);
+ if (exists && bind)
+ return 0;
+
+ if (!exists) {
+ ret = tcf_hash_create(tn, parm->index, est, a,
+ &act_sample_ops, bind, false);
+ if (ret)
+ return ret;
+ ret = ACT_P_CREATED;
+ } else {
+ tcf_hash_release(*a, bind);
+ if (!ovr)
+ return -EEXIST;
+ }
+ s = to_sample(*a);
+
+ ASSERT_RTNL();
+ s->tcf_action = parm->action;
+ s->rate = nla_get_u32(tb[TCA_SAMPLE_RATE]);
+ s->mark = nla_get_u32(tb[TCA_SAMPLE_MARK]);
+ s->eth_type = nla_get_u16(tb[TCA_SAMPLE_ETH_TYPE]);
+
+ s->truncate = tb[TCA_SAMPLE_TRUNC_SIZE];
+ if (tb[TCA_SAMPLE_TRUNC_SIZE])
+ s->trunc_size = nla_get_u32(tb[TCA_SAMPLE_TRUNC_SIZE]);
+
+ if (tb[TCA_SAMPLE_ETH_SRC])
+ ether_addr_copy(s->eth_src, nla_data(tb[TCA_SAMPLE_ETH_SRC]));
+ else
+ eth_zero_addr(s->eth_src);
+ if (tb[TCA_SAMPLE_ETH_DST])
+ ether_addr_copy(s->eth_dst, nla_data(tb[TCA_SAMPLE_ETH_DST]));
+ else
+ eth_zero_addr(s->eth_dst);
+
+ if (ret == ACT_P_CREATED)
+ tcf_hash_insert(tn, *a);
+ return ret;
+}
+
+static bool dev_ok_push(struct net_device *dev)
+{
+ switch (dev->type) {
+ case ARPHRD_TUNNEL:
+ case ARPHRD_TUNNEL6:
+ case ARPHRD_SIT:
+ case ARPHRD_IPGRE:
+ case ARPHRD_VOID:
+ case ARPHRD_NONE:
+ return false;
+ default:
+ return true;
+ }
+}
+
+static int tcf_sample(struct sk_buff *skb, const struct tc_action *a,
+ struct tcf_result *res)
+{
+ struct tcf_sample *s = to_sample(a);
+ static struct ethhdr *ethhdr;
+ struct sk_buff *skb2;
+ int orig_size;
+ int retval;
+ u32 at;
+
+ tcf_lastuse_update(&s->tcf_tm);
+ bstats_cpu_update(this_cpu_ptr(s->common.cpu_bstats), skb);
+
+ rcu_read_lock();
+ retval = READ_ONCE(s->tcf_action);
+
+ /* randomly sample packets according to rate */
+ if (prandom_u32() % s->rate == 0) {
+ skb2 = skb_copy(skb, GFP_ATOMIC);
+ if (!skb2)
+ goto out;
+
+ if (s->truncate)
+ skb_trim(skb2, s->trunc_size);
+
+ at = G_TC_AT(skb->tc_verd);
+ skb2->mac_len = skb->mac_len;
+
+ /* on ingress, the mac header gets poped, so push it back */
+ if (!(at & AT_EGRESS) && dev_ok_push(skb->dev))
+ skb_push(skb2, skb2->mac_len);
+
+ orig_size = skb->len + skb->dev->hard_header_len;
+ ethhdr = ife_packet_info_pack(skb2, orig_size,
+ skb->dev->ifindex, 0);
+ if (!ethhdr)
+ goto out;
+
+ ethhdr->h_proto = htons(s->eth_type);
+ if (!is_zero_ether_addr(s->eth_src))
+ ether_addr_copy(ethhdr->h_source, s->eth_src);
+ if (!is_zero_ether_addr(s->eth_dst))
+ ether_addr_copy(ethhdr->h_dest, s->eth_dst);
+
+ skb2->mark = s->mark;
+ netif_receive_skb(skb2);
+
+ /* mirror is always swallowed */
+ skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
+ }
+out:
+ rcu_read_unlock();
+
+ return retval;
+}
+
+static int tcf_sample_dump(struct sk_buff *skb, struct tc_action *a,
+ int bind, int ref)
+{
+ unsigned char *b = skb_tail_pointer(skb);
+ struct tcf_sample *s = to_sample(a);
+ struct tc_sample opt = {
+ .index = s->tcf_index,
+ .action = s->tcf_action,
+ .refcnt = s->tcf_refcnt - ref,
+ .bindcnt = s->tcf_bindcnt - bind,
+ };
+ struct tcf_t t;
+
+ if (nla_put(skb, TCA_SAMPLE_PARMS, sizeof(opt), &opt))
+ goto nla_put_failure;
+
+ tcf_tm_dump(&t, &s->tcf_tm);
+ if (nla_put_64bit(skb, TCA_SAMPLE_TM, sizeof(t), &t, TCA_SAMPLE_PAD))
+ goto nla_put_failure;
+
+ if (nla_put_u32(skb, TCA_SAMPLE_RATE, s->rate))
+ goto nla_put_failure;
+
+ if (nla_put_u32(skb, TCA_SAMPLE_MARK, s->mark))
+ goto nla_put_failure;
+
+ if (nla_put_u32(skb, TCA_SAMPLE_ETH_TYPE, s->eth_type))
+ goto nla_put_failure;
+
+ if (s->truncate)
+ if (nla_put_u32(skb, TCA_SAMPLE_TRUNC_SIZE, s->trunc_size))
+ goto nla_put_failure;
+
+ if (!is_zero_ether_addr(s->eth_src))
+ if (nla_put(skb, TCA_SAMPLE_ETH_SRC, ETH_ALEN, s->eth_src))
+ goto nla_put_failure;
+
+ if (!is_zero_ether_addr(s->eth_dst))
+ if (nla_put(skb, TCA_SAMPLE_ETH_DST, ETH_ALEN, s->eth_dst))
+ goto nla_put_failure;
+
+ return skb->len;
+
+nla_put_failure:
+ nlmsg_trim(skb, b);
+ return -1;
+}
+
+static int tcf_sample_walker(struct net *net, struct sk_buff *skb,
+ struct netlink_callback *cb, int type,
+ const struct tc_action_ops *ops)
+{
+ struct tc_action_net *tn = net_generic(net, sample_net_id);
+
+ return tcf_generic_walker(tn, skb, cb, type, ops);
+}
+
+static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index)
+{
+ struct tc_action_net *tn = net_generic(net, sample_net_id);
+
+ return tcf_hash_search(tn, a, index);
+}
+
+static struct tc_action_ops act_sample_ops = {
+ .kind = "sample",
+ .type = TCA_ACT_SAMPLE,
+ .owner = THIS_MODULE,
+ .act = tcf_sample,
+ .dump = tcf_sample_dump,
+ .init = tcf_sample_init,
+ .walk = tcf_sample_walker,
+ .lookup = tcf_sample_search,
+ .size = sizeof(struct tcf_sample),
+};
+
+static __net_init int sample_init_net(struct net *net)
+{
+ struct tc_action_net *tn = net_generic(net, sample_net_id);
+
+ return tc_action_net_init(tn, &act_sample_ops, SAMPLE_TAB_MASK);
+}
+
+static void __net_exit sample_exit_net(struct net *net)
+{
+ struct tc_action_net *tn = net_generic(net, sample_net_id);
+
+ tc_action_net_exit(tn);
+}
+
+static struct pernet_operations sample_net_ops = {
+ .init = sample_init_net,
+ .exit = sample_exit_net,
+ .id = &sample_net_id,
+ .size = sizeof(struct tc_action_net),
+};
+
+static int __init sample_init_module(void)
+{
+ return tcf_register_action(&act_sample_ops, &sample_net_ops);
+}
+
+static void __exit sample_cleanup_module(void)
+{
+ tcf_unregister_action(&act_sample_ops, &sample_net_ops);
+}
+
+module_init(sample_init_module);
+module_exit(sample_cleanup_module);
+
+MODULE_AUTHOR("Yotam Gigi <yotamg@mellanox.com>");
+MODULE_DESCRIPTION("Packet sampling action");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* [patch net-next v2 4/8] net: ife: Introduce packet info packing method
From: Jiri Pirko @ 2016-11-14 15:00 UTC (permalink / raw)
To: netdev
Cc: davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
geert+renesas, stephen, xiyou.wangcong, linux, roopa,
john.fastabend, simon.horman
In-Reply-To: <1479135638-3580-1-git-send-email-jiri@resnulli.us>
From: Yotam Gigi <yotamg@mellanox.com>
Add the ife_packet_info_pack function which encodes some informative
metadata into a packet using the ife encapsulation. The informative
metadata consists of:
- The packet input/output ifindices
- The packet original size, in case it was truncated
This method is useful for anyone that wants to pass informative metadata
on a packet alongside with the packet itself. A good usage can be packet
sampling.
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/ife.h | 10 ++++++++++
net/ife/ife.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+)
diff --git a/include/net/ife.h b/include/net/ife.h
index a75d4e0..5fbcfb3 100644
--- a/include/net/ife.h
+++ b/include/net/ife.h
@@ -18,6 +18,9 @@ int ife_tlv_meta_encode(void *skbdata, u16 attrtype, u16 dlen,
void *ife_tlv_meta_next(void *skbdata);
+struct ethhdr *ife_packet_info_pack(struct sk_buff *skb, int orig_size,
+ int in_ifindex, int out_ifindex);
+
#else
static inline void *ife_encode(struct sk_buff *skb, u16 metalen)
@@ -47,6 +50,13 @@ static inline void *ife_tlv_meta_next(void *skbdata)
return NULL;
}
+static inline struct ethhdr *
+ife_packet_info_pack(struct sk_buff *skb, int orig_size, int in_ifindex,
+ int out_ifindex)
+{
+ return NULL;
+}
+
#endif
#endif /* __NET_IFE_H */
diff --git a/net/ife/ife.c b/net/ife/ife.c
index 3642f43..f0b683e 100644
--- a/net/ife/ife.c
+++ b/net/ife/ife.c
@@ -138,6 +138,51 @@ int ife_tlv_meta_encode(void *skbdata, u16 attrtype, u16 dlen, const void *dval)
}
EXPORT_SYMBOL_GPL(ife_tlv_meta_encode);
+struct ethhdr *ife_packet_info_pack(struct sk_buff *skb, int orig_size,
+ int in_ifindex, int out_ifindex)
+{
+ int curr_size;
+ void *ifetlv;
+ u16 metalen;
+
+ curr_size = skb->len;
+
+ metalen = nla_total_size(sizeof(orig_size)) +
+ nla_total_size(sizeof(curr_size));
+
+ if (in_ifindex)
+ metalen += nla_total_size(sizeof(in_ifindex));
+ if (out_ifindex)
+ metalen += nla_total_size(sizeof(out_ifindex));
+
+ in_ifindex = htonl(in_ifindex);
+ out_ifindex = htonl(out_ifindex);
+ orig_size = htonl(orig_size);
+ curr_size = htonl(curr_size);
+
+ ifetlv = ife_encode(skb, metalen);
+ if (!ifetlv)
+ return NULL;
+
+ if (in_ifindex)
+ ifetlv += ife_tlv_meta_encode(ifetlv, IFE_META_IN_IFINDEX,
+ sizeof(in_ifindex), &in_ifindex);
+
+ if (out_ifindex)
+ ifetlv += ife_tlv_meta_encode(ifetlv, IFE_META_OUT_IFINDEX,
+ sizeof(out_ifindex),
+ &out_ifindex);
+
+ ifetlv += ife_tlv_meta_encode(ifetlv, IFE_META_ORIGSIZE,
+ sizeof(orig_size), &orig_size);
+
+ ifetlv += ife_tlv_meta_encode(ifetlv, IFE_META_SIZE,
+ sizeof(curr_size), &curr_size);
+
+ return (struct ethhdr *) skb->data;
+}
+EXPORT_SYMBOL(ife_packet_info_pack);
+
MODULE_AUTHOR("Jamal Hadi Salim <jhs@mojatatu.com>");
MODULE_AUTHOR("Yotam Gigi <yotamg@mellanox.com>");
MODULE_DESCRIPTION("Inter-FE LFB action");
--
2.7.4
^ permalink raw reply related
* [patch net-next v2 3/8] net: ife: Introduce new metadata tlv types
From: Jiri Pirko @ 2016-11-14 15:00 UTC (permalink / raw)
To: netdev
Cc: davem, yotamg, idosch, eladr, nogahf, ogerlitz, jhs,
geert+renesas, stephen, xiyou.wangcong, linux, roopa,
john.fastabend, simon.horman
In-Reply-To: <1479135638-3580-1-git-send-email-jiri@resnulli.us>
From: Yotam Gigi <yotamg@mellanox.com>
IFE_META_IN_IFINDEX: Allow to pass input ifindex value as part of the
ife metadata
IFE_META_OUT_IFINDEX: Allow to pass output ifindex value as part of the
ife metadata
IFE_META_ORIG_SIZE: Allow to pass the original packet size as part of
the ife metadata. Can be used in case that the packet is truncated
IFE_META_SIZE: Allow to pass the size of the encapsulated packet as
part of the ife metadata
Signed-off-by: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/uapi/linux/ife.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/uapi/linux/ife.h b/include/uapi/linux/ife.h
index 2954da3..ebdd785 100644
--- a/include/uapi/linux/ife.h
+++ b/include/uapi/linux/ife.h
@@ -9,6 +9,10 @@ enum {
IFE_META_PRIO,
IFE_META_QMAP,
IFE_META_TCINDEX,
+ IFE_META_IN_IFINDEX,
+ IFE_META_OUT_IFINDEX,
+ IFE_META_ORIGSIZE,
+ IFE_META_SIZE,
__IFE_META_MAX
};
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.