* [PATCH net-next] Revert "net: sched: route: don't set arg->stop in route4_walk() when empty"
From: Vlad Buslov @ 2019-02-27 13:48 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, Vlad Buslov
This reverts commit 3027ff41f67c ("net: sched: route: don't set arg->stop
in route4_walk() when empty")
Cls API function tcf_proto_is_empty() was changed in commit
6676d5e416ee ("net: sched: set dedicated tcf_walker flag when tp is empty")
to no longer depend on arg->stop to determine that classifier instance is
empty. Instead, it adds dedicated arg->nonempty field, which makes the fix
in route classifier no longer necessary.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/cls_route.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index f006af23b64a..319f68cc233f 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -567,7 +567,10 @@ static void route4_walk(struct tcf_proto *tp, struct tcf_walker *arg,
struct route4_head *head = rtnl_dereference(tp->root);
unsigned int h, h1;
- if (head == NULL || arg->stop)
+ if (head == NULL)
+ arg->stop = 1;
+
+ if (arg->stop)
return;
for (h = 0; h <= 256; h++) {
--
2.13.6
^ permalink raw reply related
* [PATCH net-next] Revert "net: sched: fw: don't set arg->stop in fw_walk() when empty"
From: Vlad Buslov @ 2019-02-27 13:49 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, Vlad Buslov
This reverts commit 31a998487641 ("net: sched: fw: don't set arg->stop in
fw_walk() when empty")
Cls API function tcf_proto_is_empty() was changed in commit
6676d5e416ee ("net: sched: set dedicated tcf_walker flag when tp is empty")
to no longer depend on arg->stop to determine that classifier instance is
empty. Instead, it adds dedicated arg->nonempty field, which makes the fix
in fw classifier no longer necessary.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/cls_fw.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 456ee6e62dfa..ad036b00427d 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -363,7 +363,10 @@ static void fw_walk(struct tcf_proto *tp, struct tcf_walker *arg,
struct fw_head *head = rtnl_dereference(tp->root);
int h;
- if (head == NULL || arg->stop)
+ if (head == NULL)
+ arg->stop = 1;
+
+ if (arg->stop)
return;
for (h = 0; h < HTSIZE; h++) {
--
2.13.6
^ permalink raw reply related
* Re: [PATCH][net-next] ethtool: Use explicit designated initializers for .cmd
From: Michal Kubecek @ 2019-02-27 13:51 UTC (permalink / raw)
To: Li RongQing; +Cc: netdev
In-Reply-To: <1551271677-14492-1-git-send-email-lirongqing@baidu.com>
On Wed, Feb 27, 2019 at 08:47:57PM +0800, Li RongQing wrote:
> Initialize the .cmd member by using a designated struct
> initializer. This fixes warning of missing field initializers,
> and makes code a little easier to read.
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
> ---
> net/core/ethtool.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 47558ffae423..d4918ffddda8 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1717,7 +1717,7 @@ static noinline_for_stack int ethtool_set_channels(struct net_device *dev,
>
> static int ethtool_get_pauseparam(struct net_device *dev, void __user *useraddr)
> {
> - struct ethtool_pauseparam pauseparam = { ETHTOOL_GPAUSEPARAM };
> + struct ethtool_pauseparam pauseparam = { .cmd = ETHTOOL_GPAUSEPARAM };
>
> if (!dev->ethtool_ops->get_pauseparam)
> return -EOPNOTSUPP;
> @@ -2503,7 +2503,7 @@ static int set_phy_tunable(struct net_device *dev, void __user *useraddr)
>
> static int ethtool_get_fecparam(struct net_device *dev, void __user *useraddr)
> {
> - struct ethtool_fecparam fecparam = { ETHTOOL_GFECPARAM };
> + struct ethtool_fecparam fecparam = { .cmd = ETHTOOL_GFECPARAM };
> int rc;
>
> if (!dev->ethtool_ops->get_fecparam)
> --
> 2.16.2
>
^ permalink raw reply
* Re: [PATCH net-next v3 4/4] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action
From: Roi Dayan @ 2019-02-27 14:01 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org
In-Reply-To: <1551184063-40628-5-git-send-email-xiangxia.m.yue@gmail.com>
On 26/02/2019 14:27, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> * Now the encapsulation is not supported for mlx5 VFs. When we try to
> offload that action, the -EINVAL is returned, but not -EOPNOTSUPP.
> This patch changes the returned value and ignore to confuse user.
> The command is shown as below [1].
>
> * When max modify header action is zero, we return -EOPNOTSUPP
> directly. In this way, we can ignore wrong message info (e.g.
> "mlx5: parsed 0 pedit actions, can't do more"). This happens when
> offloading pedit actions on mlx(cx4) VFs. The command is shown as below [2].
>
> For example: (p2p1_0 is VF net device)
> [1]
> $ tc filter add dev p2p1_0 protocol ip parent ffff: prio 1 flower skip_sw \
> src_mac e4:11:22:33:44:01 \
> action tunnel_key set \
> src_ip 1.1.1.100 \
> dst_ip 1.1.1.200 \
> dst_port 4789 id 100 \
> action mirred egress redirect dev vxlan0
>
> [2]
> $ tc filter add dev p2p1_0 parent ffff: protocol ip prio 1 \
> flower skip_sw dst_mac 00:10:56:fb:64:e8 \
> dst_ip 1.1.1.100 src_ip 1.1.1.200 \
> action pedit ex munge eth src set 00:10:56:b4:5d:20
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 56ac50d..3a02b22 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2035,7 +2035,7 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
> struct netlink_ext_ack *extack)
> {
> u8 cmd = (act->id == FLOW_ACTION_MANGLE) ? 0 : 1;
> - int err = -EOPNOTSUPP;
> + int max_actions, err = -EOPNOTSUPP;
> u32 mask, val, offset;
> u8 htype;
>
> @@ -2047,6 +2047,17 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
> goto out_err;
> }
>
> + if (namespace == MLX5_FLOW_NAMESPACE_FDB) /* FDB offloading */
> + max_actions = MLX5_CAP_ESW_FLOWTABLE_FDB(priv->mdev, max_modify_header_actions);
> + else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
> + max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
> +
we have the exact same block with comments in alloc_mod_hdr_actions() which
is called later after we parse the action and now parsing the pedit fields.
your check for max_actions was there in prev v. why move it?
if like this I suggest we create a small static function to get max actions
for a namespace and call it from both places to get max actions.
> + if (!max_actions) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "don't support pedit actions, can't offload");
please rephrase the msg to be consistent
i.e. The pedit offload action is not supported
> + goto out_err;
> + }
> +
> mask = act->mangle.mask;
> val = act->mangle.val;
> offset = act->mangle.offset;
> @@ -2294,7 +2305,8 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv,
> }
> break;
> default:
> - return -EINVAL;
> + NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
> + return -EOPNOTSUPP;
> }
> }
>
> @@ -2616,7 +2628,8 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
> break;
> }
> default:
> - return -EINVAL;
> + NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
> + return -EOPNOTSUPP;
> }
> }
>
>
^ permalink raw reply
* Re: [PATCH] can: flexcan: add TX support for variable payload size
From: Marc Kleine-Budde @ 2019-02-27 14:03 UTC (permalink / raw)
To: Joakim Zhang, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <20181212064419.18613-1-qiangqing.zhang@nxp.com>
[-- Attachment #1.1: Type: text/plain, Size: 4992 bytes --]
On 12/12/18 7:46 AM, Joakim Zhang wrote:
> Now the FlexCAN driver always use last mailbox for TX, it will work well
> when MB payload size is 8/16 bytes.
> TX mailbox would change to 13 when MB payload size is 64 bytes to
> support CANFD. So we may need to set iflag register to add support for
> variable payload size.
>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> drivers/net/can/flexcan.c | 42 +++++++++++++++++++++++++++++----------
> 1 file changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 0f36eafe3ac1..13fd085fcf84 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -141,7 +141,9 @@
> #define FLEXCAN_TX_MB_RESERVED_OFF_FIFO 8
> #define FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP 0
> #define FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST (FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP + 1)
> -#define FLEXCAN_IFLAG_MB(x) BIT((x) & 0x1f)
> +#define FLEXCAN_IFLAG1_MB_NUM 32
> +#define FLEXCAN_IFLAG1_MB(x) BIT(x)
> +#define FLEXCAN_IFLAG2_MB(x) BIT((x) & 0x1f)
> #define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW BIT(7)
> #define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
> #define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE BIT(5)
> @@ -822,9 +824,15 @@ static inline u64 flexcan_read_reg_iflag_rx(struct flexcan_priv *priv)
> struct flexcan_regs __iomem *regs = priv->regs;
> u32 iflag1, iflag2;
>
> - iflag2 = priv->read(®s->iflag2) & priv->reg_imask2_default &
> - ~FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
> - iflag1 = priv->read(®s->iflag1) & priv->reg_imask1_default;
> + if (priv->tx_mb_idx >= FLEXCAN_IFLAG1_MB_NUM) {
> + iflag2 = priv->read(®s->iflag2) & priv->reg_imask2_default &
> + ~FLEXCAN_IFLAG2_MB(priv->tx_mb_idx);
> + iflag1 = priv->read(®s->iflag1) & priv->reg_imask1_default;
> + } else {
> + iflag2 = priv->read(®s->iflag2) & priv->reg_imask2_default;
> + iflag1 = priv->read(®s->iflag1) & priv->reg_imask1_default &
> + ~FLEXCAN_IFLAG1_MB(priv->tx_mb_idx);
> + }
I just noticed, that FLEXCAN_IFLAGx_MB(priv->tx_mb_idx) should already
be part of the corresponding imaskx_default. See flexcan_open(). So we
can remove it completely here, right?
>
> return (u64)iflag2 << 32 | iflag1;
> }
> @@ -836,7 +844,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> struct flexcan_priv *priv = netdev_priv(dev);
> struct flexcan_regs __iomem *regs = priv->regs;
> irqreturn_t handled = IRQ_NONE;
> - u32 reg_iflag2, reg_esr;
> + u32 reg_tx_iflag, tx_iflag_idx, reg_esr;
"tx_iflag_idx" is not an index (going from 0...63) but a bit-mask.
> + void __iomem *reg_iflag;
"reg_iflag" is not a register but a pointer to a register, better rename
it. There is a "u64 reg_iflag" in the same function already, but in a
different scope. Why not make it a u32 instead of a void?
> enum can_state last_state = priv->can.state;
>
> /* reception interrupt */
> @@ -870,10 +879,18 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> }
> }
>
> - reg_iflag2 = priv->read(®s->iflag2);
> + if (priv->tx_mb_idx >= FLEXCAN_IFLAG1_MB_NUM) {
> + reg_tx_iflag = priv->read(®s->iflag2);
> + tx_iflag_idx = FLEXCAN_IFLAG2_MB(priv->tx_mb_idx);
> + reg_iflag = ®s->iflag2;
> + } else {
> + reg_tx_iflag = priv->read(®s->iflag1);
> + tx_iflag_idx = FLEXCAN_IFLAG1_MB(priv->tx_mb_idx);
> + reg_iflag = ®s->iflag1;
> + }
>
> /* transmission complete interrupt */
> - if (reg_iflag2 & FLEXCAN_IFLAG_MB(priv->tx_mb_idx)) {
> + if (reg_tx_iflag & tx_iflag_idx) {
> u32 reg_ctrl = priv->read(&priv->tx_mb->can_ctrl);
>
> handled = IRQ_HANDLED;
> @@ -885,7 +902,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> /* after sending a RTR frame MB is in RX mode */
> priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> &priv->tx_mb->can_ctrl);
> - priv->write(FLEXCAN_IFLAG_MB(priv->tx_mb_idx), ®s->iflag2);
> + priv->write(tx_iflag_idx, reg_iflag);
> netif_wake_queue(dev);
> }
>
> @@ -1244,8 +1261,13 @@ static int flexcan_open(struct net_device *dev)
> priv->tx_mb_idx = priv->mb_count - 1;
> priv->tx_mb = flexcan_get_mb(priv, priv->tx_mb_idx);
>
> - priv->reg_imask1_default = 0;
> - priv->reg_imask2_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
> + if (priv->tx_mb_idx >= FLEXCAN_IFLAG1_MB_NUM) {
> + priv->reg_imask1_default = 0;
> + priv->reg_imask2_default = FLEXCAN_IFLAG2_MB(priv->tx_mb_idx);
> + } else {
> + priv->reg_imask1_default = FLEXCAN_IFLAG1_MB(priv->tx_mb_idx);
> + priv->reg_imask2_default = 0;
> + }
>
> priv->offload.mailbox_read = flexcan_mailbox_read;
>
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net v1] net/mlx5e: Correctly use the namespace type when allocating pedit action
From: Roi Dayan @ 2019-02-27 14:05 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org, Pablo Neira Ayuso
In-Reply-To: <1551184112-40686-1-git-send-email-xiangxia.m.yue@gmail.com>
On 26/02/2019 14:28, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> The capacity of FDB offloading and NIC offloading table are
> different, and when allocating the pedit actions, we should
> use the correct namespace type.
>
> Fixes: c500c86b0c75d ("net/mlx5e: support for two independent packet edit actions")
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 3a02b22..467ef9e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2635,7 +2635,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
>
> if (hdrs[TCA_PEDIT_KEY_EX_CMD_SET].pedits ||
> hdrs[TCA_PEDIT_KEY_EX_CMD_ADD].pedits) {
> - err = alloc_tc_pedit_action(priv, MLX5_FLOW_NAMESPACE_KERNEL,
> + err = alloc_tc_pedit_action(priv, MLX5_FLOW_NAMESPACE_FDB,
> parse_attr, hdrs, extack);
> if (err)
> return err;
>
Reviewed-by: Roi Dayan <roid@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next] net: sched: set dedicated tcf_walker flag when tp is empty
From: Vlad Buslov @ 2019-02-27 14:28 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <CAM_iQpVjFKwwniiFKy5ocRQ-HB=xzm-Lj6pGihFGXV6Cwj8TVQ@mail.gmail.com>
On Tue 26 Feb 2019 at 22:38, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Feb 26, 2019 at 7:08 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>>
>> On Mon 25 Feb 2019 at 22:52, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> > On Mon, Feb 25, 2019 at 7:38 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> >>
>> >> Using tcf_walker->stop flag to determine when tcf_walker->fn() was called
>> >> at least once is unreliable. Some classifiers set 'stop' flag on error
>> >> before calling walker callback, other classifiers used to call it with NULL
>> >> filter pointer when empty. In order to prevent further regressions, extend
>> >> tcf_walker structure with dedicated 'nonempty' flag. Set this flag in
>> >> tcf_walker->fn() implementation that is used to check if classifier has
>> >> filters configured.
>> >
>> >
>> > So, after this patch commits like 31a998487641 ("net: sched: fw: don't
>> > set arg->stop in fw_walk() when empty") can be reverted??
>>
>> Yes, it is safe now to revert following commits:
>>
>> 3027ff41f67c ("net: sched: route: don't set arg->stop in route4_walk() when empty")
>> 31a998487641 ("net: sched: fw: don't set arg->stop in fw_walk() when empty")
>
> Yeah, and probably commit d66022cd1623
> ("net: sched: matchall: verify that filter is not NULL in mall_walk()").
>
> Please send a patch to revert them all.
>
> Thanks.
I think commit d66022cd1623 ("net: sched: matchall: verify that filter
is not NULL in mall_walk()") and commit 8b58d12f4ae1 ("net: sched:
cgroup: verify that filter is not NULL during walk") shouldn't be
reverted. They are still necessary to prevent tcf_chain_dump() from
dumping NULL filter pointer. It can happen when dump is initiated in
parallel with inserting first filter to unlocked classifier.
tcf_fill_node() verifies that filter pointer is not NULL, so it will not
crash, but will output tcf_proto info for second time. This might
"confuse" user-space.
What do you think?
^ permalink raw reply
* [PATCH] bpf: drop refcount if bpf_map_new_fd() fails in map_create()
From: zerons @ 2019-02-27 14:36 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, linux-kernel, Peng Sun
In bpf/syscall.c, map_create() first set map->usercnt to 1, a file descriptor is
supposed to return to userspace. When bpf_map_new_fd() fails, drop the refcount.
Signed-off-by: Peng Sun <sironhide0null@gmail.com>
---
kernel/bpf/syscall.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cf5040f..1c4f1c4 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -558,12 +558,12 @@ static int map_create(union bpf_attr *attr)
err = bpf_map_new_fd(map, f_flags);
if (err < 0) {
/* failed to allocate fd.
- * bpf_map_put() is needed because the above
+ * bpf_map_put_with_uref() is needed because the above
* bpf_map_alloc_id() has published the map
* to the userspace and the userspace may
* have refcnt-ed it through BPF_MAP_GET_FD_BY_ID.
*/
- bpf_map_put(map);
+ bpf_map_put_with_uref(map);
return err;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] dt-bindings: net: can: binding for CTU CAN FD open-source IP core.
From: Marc Kleine-Budde @ 2019-02-27 14:47 UTC (permalink / raw)
To: Pavel Pisa, devicetree
Cc: Wolfgang Grandegger, David S. Miller, Rob Herring, Mark Rutland,
linux-can, netdev, linux-kernel, Martin Jerabek, Ondrej Ille
In-Reply-To: <201902221420.05267.pisa@cmp.felk.cvut.cz>
[-- Attachment #1.1: Type: text/plain, Size: 5286 bytes --]
On 2/22/19 2:20 PM, Pavel Pisa wrote:
> From 3e19a7f5c33e5fb50f52c9df05bf00022e3f3dd5 Mon Sep 17 00:00:00 2001
> From: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> Date: Fri, 22 Feb 2019 14:11:11 +0100
> Subject: [PATCH] dt-bindings: net: can: binding for CTU CAN FD open-source IP
> core.
Nice! I appreciate open-source hardware!
Please send the patch via git send-email, it's a bit broken:
Applying: dt-bindings: net: can: binding for CTU CAN FD open-source IP core.
.git/rebase-apply/patch:29: trailing whitespace.
error: corrupt patch at line 30
Patch failed at 0001 dt-bindings: net: can: binding for CTU CAN FD
open-source IP core.
hint: Use 'git am --show-current-patch' to see the failed patch
> Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
> ---
> .../devicetree/bindings/net/can/ctu,ctucanfd.txt | 102 +++++++++++++++++++++
> 1 file changed, 102 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt
>
> diff --git a/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt
> b/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt
> new file mode 100644
> index 000000000000..6c75e5850904
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/ctu,ctucanfd.txt
> @@ -0,0 +1,102 @@
> +Memory mapped CTU CAN FD open-source IP core
> +
> +The core sources and documentation on project page
> +
> + https://gitlab.fel.cvut.cz/canbus/ctucanfd_ip_core
> + http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/Progdokum.pdf
> +
> +Integration in Xilinx Zynq SoC based system together with
> +OpenCores SJA1000 compatible controllers
> +
> + https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top
> +
> +Martin Jerabek's dimploma thesis with integration and testing
> +framework description
> +
> +
> https://dspace.cvut.cz/bitstream/handle/10467/80366/F3-DP-2019-Jerabek-Martin-Jerabek-thesis-2019-canfd.pdf
> +
> +Required properties:
> +
> +- compatible : should be one of "ctu,ctucanfd", "ctu,canfd-2".
> +
> +- reg = <(baseaddr) (size)> : specify mapping into physical address
> + space of the processor system.
> +
> +- interrupts : property with a value describing the interrupt source
> + required for the CTU CAN FD. For Zynq SoC system format is
> + <(is_spi) (number) (type)> where is_spi defines if it is SPI
> + (shared peripheral) interrupt, the second number is translated
> + to the vector by addition of 32 on Zynq-7000 systems and type
> + is IRQ_TYPE_LEVEL_HIGH (4) for Zynq.
> +
> +- interrupt-parent = <&interrupt-controller-phandle> :
> + is required for Zynq SoC to find map interrupt
> + to the correct controller
> +
> +- clocks: phandle of reference clock (100 MHz is appropriate
> + for FPGA implementation on Zynq-7000 system).
> +
> +Example when integrated to Zynq-7000 system DTS:
> +
> + / {
> + /* ... */
> + amba: amba {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "simple-bus";
> +
> + CTU_CAN_FD_0: CTU_CAN_FD@43c30000 {
It's unusual to have uppercase letters here.
> + compatible = "ctu,ctucanfd";
Is ctu already registered in
Documentation/devicetree/bindings/vendor-prefixes.txt ?
> + interrupt-parent = <&intc>;
> + interrupts = <0 30 4>;
> + clocks = <&clkc 15>;
> + reg = <0x43c30000 0x10000>;
> + };
> + };
> + };
> +
> +
> +Example when used as DTS overlay on Zynq-7000 system:
> +
> +
> +// Device Tree Example: Full Reconfiguration without Bridges
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> + fragment@0 {
> + target-path = "/fpga-full";
> +
> + __overlay__ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + firmware-name = "system.bit.bin";
> + };
> + };
> +
> + fragment@1 {
> + target-path = "/amba";
> + __overlay__ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + CTU_CAN_FD_0: CTU_CAN_FD@43c30000 {
> + compatible = "ctu,ctucanfd";
> + interrupt-parent = <&intc>;
> + interrupts = <0 30 4>;
> + clocks = <&clkc 15>;
> + //clock-names = "can_clk";
Is this needed or not?
> + reg = <0x43c30000 0x10000>;
> + };
> + CTU_CAN_FD_1: CTU_CAN_FD@43c70000 {
> + compatible = "ctu,ctucanfd";
> + interrupt-parent = <&intc>;
> + interrupts = <0 31 4>;
> + clocks = <&clkc 15>;
> + //clock-names = "can_clk";
Is this needed or not?
> + reg = <0x43c70000 0x10000>;
> + };
> + };
> + };
> +};
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] appletalk: Fix use-after-free in atalk_proc_exit
From: Yue Haibing @ 2019-02-27 14:59 UTC (permalink / raw)
To: davem, joe, gregkh; +Cc: linux-kernel, netdev, YueHaibing
From: YueHaibing <yuehaibing@huawei.com>
KASAN report this:
BUG: KASAN: use-after-free in pde_subdir_find+0x12d/0x150 fs/proc/generic.c:71
Read of size 8 at addr ffff8881f41fe5b0 by task syz-executor.0/2806
CPU: 0 PID: 2806 Comm: syz-executor.0 Not tainted 5.0.0-rc7+ #45
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xfa/0x1ce lib/dump_stack.c:113
print_address_description+0x65/0x270 mm/kasan/report.c:187
kasan_report+0x149/0x18d mm/kasan/report.c:317
pde_subdir_find+0x12d/0x150 fs/proc/generic.c:71
remove_proc_entry+0xe8/0x420 fs/proc/generic.c:667
atalk_proc_exit+0x18/0x820 [appletalk]
atalk_exit+0xf/0x5a [appletalk]
__do_sys_delete_module kernel/module.c:1018 [inline]
__se_sys_delete_module kernel/module.c:961 [inline]
__x64_sys_delete_module+0x3dc/0x5e0 kernel/module.c:961
do_syscall_64+0x147/0x600 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x462e99
Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fb2de6b9c58 EFLAGS: 00000246 ORIG_RAX: 00000000000000b0
RAX: ffffffffffffffda RBX: 000000000073bf00 RCX: 0000000000462e99
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000200001c0
RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fb2de6ba6bc
R13: 00000000004bccaa R14: 00000000006f6bc8 R15: 00000000ffffffff
Allocated by task 2806:
set_track mm/kasan/common.c:85 [inline]
__kasan_kmalloc.constprop.3+0xa0/0xd0 mm/kasan/common.c:496
slab_post_alloc_hook mm/slab.h:444 [inline]
slab_alloc_node mm/slub.c:2739 [inline]
slab_alloc mm/slub.c:2747 [inline]
kmem_cache_alloc+0xcf/0x250 mm/slub.c:2752
kmem_cache_zalloc include/linux/slab.h:730 [inline]
__proc_create+0x30f/0xa20 fs/proc/generic.c:408
proc_mkdir_data+0x47/0x190 fs/proc/generic.c:469
0xffffffffc10c01bb
0xffffffffc10c0166
do_one_initcall+0xfa/0x5ca init/main.c:887
do_init_module+0x204/0x5f6 kernel/module.c:3460
load_module+0x66b2/0x8570 kernel/module.c:3808
__do_sys_finit_module+0x238/0x2a0 kernel/module.c:3902
do_syscall_64+0x147/0x600 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 2806:
set_track mm/kasan/common.c:85 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/common.c:458
slab_free_hook mm/slub.c:1409 [inline]
slab_free_freelist_hook mm/slub.c:1436 [inline]
slab_free mm/slub.c:2986 [inline]
kmem_cache_free+0xa6/0x2a0 mm/slub.c:3002
pde_put+0x6e/0x80 fs/proc/generic.c:647
remove_proc_entry+0x1d3/0x420 fs/proc/generic.c:684
0xffffffffc10c031c
0xffffffffc10c0166
do_one_initcall+0xfa/0x5ca init/main.c:887
do_init_module+0x204/0x5f6 kernel/module.c:3460
load_module+0x66b2/0x8570 kernel/module.c:3808
__do_sys_finit_module+0x238/0x2a0 kernel/module.c:3902
do_syscall_64+0x147/0x600 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8881f41fe500
which belongs to the cache proc_dir_entry of size 256
The buggy address is located 176 bytes inside of
256-byte region [ffff8881f41fe500, ffff8881f41fe600)
The buggy address belongs to the page:
page:ffffea0007d07f80 count:1 mapcount:0 mapping:ffff8881f6e69a00 index:0x0
flags: 0x2fffc0000000200(slab)
raw: 02fffc0000000200 dead000000000100 dead000000000200 ffff8881f6e69a00
raw: 0000000000000000 00000000800c000c 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881f41fe480: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
ffff8881f41fe500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff8881f41fe580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881f41fe600: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
ffff8881f41fe680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
It should check the return value of atalk_proc_init fails,
otherwise atalk_exit calls pde_subdir_find will trgger
use-after-free while unloading the module.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
net/appletalk/ddp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 9b6bc5a..75075fe 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1925,7 +1925,10 @@ static int __init atalk_init(void)
register_netdevice_notifier(&ddp_notifier);
aarp_proto_init();
- atalk_proc_init();
+ rc = atalk_proc_init();
+ if (rc)
+ goto out;
+
atalk_register_sysctl();
out:
return rc;
--
2.7.0
^ permalink raw reply related
* [PATCH -next] appletalk: use remove_proc_subtree to simplify procfs code
From: Yue Haibing @ 2019-02-27 15:00 UTC (permalink / raw)
To: davem, joe, gregkh; +Cc: linux-kernel, netdev, YueHaibing
From: YueHaibing <yuehaibing@huawei.com>
Use remove_proc_subtree to remove the whole subtree
on cleanup.Also do some cleanup.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
net/appletalk/atalk_proc.c | 56 ++++++++++++++--------------------------------
1 file changed, 17 insertions(+), 39 deletions(-)
diff --git a/net/appletalk/atalk_proc.c b/net/appletalk/atalk_proc.c
index 8006295..398b767 100644
--- a/net/appletalk/atalk_proc.c
+++ b/net/appletalk/atalk_proc.c
@@ -210,56 +210,34 @@ static const struct seq_operations atalk_seq_socket_ops = {
.show = atalk_seq_socket_show,
};
-static struct proc_dir_entry *atalk_proc_dir;
-
int __init atalk_proc_init(void)
{
- struct proc_dir_entry *p;
- int rc = -ENOMEM;
+ if (!proc_mkdir("atalk", init_net.proc_net))
+ return -ENOMEM;
- atalk_proc_dir = proc_mkdir("atalk", init_net.proc_net);
- if (!atalk_proc_dir)
+ if (!proc_create_seq("atalk/interface", 0444, init_net.proc_net,
+ &atalk_seq_interface_ops));
goto out;
- p = proc_create_seq("interface", 0444, atalk_proc_dir,
- &atalk_seq_interface_ops);
- if (!p)
- goto out_interface;
-
- p = proc_create_seq("route", 0444, atalk_proc_dir,
- &atalk_seq_route_ops);
- if (!p)
- goto out_route;
+ if (!proc_create_seq("atalk/route", 0444, init_net.proc_net,
+ &atalk_seq_route_ops));
+ goto out;
- p = proc_create_seq("socket", 0444, atalk_proc_dir,
- &atalk_seq_socket_ops);
- if (!p)
- goto out_socket;
+ if (!proc_create_seq("atalk/socket", 0444, init_net.proc_net,
+ &atalk_seq_socket_ops));
+ goto out;
- p = proc_create_seq_private("arp", 0444, atalk_proc_dir, &aarp_seq_ops,
- sizeof(struct aarp_iter_state), NULL);
- if (!p)
- goto out_arp;
+ if (!proc_create_seq_private("atalk/arp", 0444, init_net.proc_net,
+ &aarp_seq_ops,
+ sizeof(struct aarp_iter_state), NULL));
+ goto out;
- rc = 0;
out:
- return rc;
-out_arp:
- remove_proc_entry("socket", atalk_proc_dir);
-out_socket:
- remove_proc_entry("route", atalk_proc_dir);
-out_route:
- remove_proc_entry("interface", atalk_proc_dir);
-out_interface:
- remove_proc_entry("atalk", init_net.proc_net);
- goto out;
+ remove_proc_subtree("atalk", init_net.proc_net);
+ return -ENOMEM;
}
void __exit atalk_proc_exit(void)
{
- remove_proc_entry("interface", atalk_proc_dir);
- remove_proc_entry("route", atalk_proc_dir);
- remove_proc_entry("socket", atalk_proc_dir);
- remove_proc_entry("arp", atalk_proc_dir);
- remove_proc_entry("atalk", init_net.proc_net);
+ remove_proc_subtree("atalk", init_net.proc_net);
}
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net 4/4] tls: Fix tls_device receive
From: Boris Pismenny @ 2019-02-27 15:23 UTC (permalink / raw)
To: Vakul Garg, Dave Watson
Cc: Aviad Yehezkel, john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org, Eran Ben Elisha
In-Reply-To: <DB7PR04MB4252D936A2C713C60542A6828B740@DB7PR04MB4252.eurprd04.prod.outlook.com>
On 2/27/2019 5:08 AM, Vakul Garg wrote:
>
>
>> -----Original Message-----
>> From: Dave Watson <davejwatson@fb.com>
>> Sent: Wednesday, February 27, 2019 2:05 AM
>> To: Boris Pismenny <borisp@mellanox.com>
>> Cc: aviadye@mellanox.com; john.fastabend@gmail.com;
>> daniel@iogearbox.net; Vakul Garg <vakul.garg@nxp.com>;
>> netdev@vger.kernel.org; eranbe@mellanox.com
>> Subject: Re: [PATCH net 4/4] tls: Fix tls_device receive
>>
>> On 02/26/19 02:12 PM, Boris Pismenny wrote:
>>> Currently, the receive function fails to handle records already
>>> decrypted by the device due to the commit mentioned below.
>>>
>>> This commit advances the TLS record sequence number and prepares the
>>> context to handle the next record.
>>>
>>> Fixes: fedf201e1296 ("net: tls: Refactor control message handling on
>>> recv")
>>> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
>>> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
>>> ---
>>> net/tls/tls_sw.c | 15 +++++++--------
>>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
>>> f515cd7e984e..85da10182d8d 100644
>>> --- a/net/tls/tls_sw.c
>>> +++ b/net/tls/tls_sw.c
>>> @@ -1481,18 +1481,17 @@ static int decrypt_skb_update(struct sock *sk,
>>> struct sk_buff *skb,
>>>
>>> return err;
>>> }
>>> -
>>> - rxm->full_len -= padding_length(ctx, tls_ctx, skb);
>>> -
>>> - rxm->offset += prot->prepend_size;
>>> - rxm->full_len -= prot->overhead_size;
>>> - tls_advance_record_sn(sk, &tls_ctx->rx, version);
>>> - ctx->decrypted = true;
>>> - ctx->saved_data_ready(sk);
>>> } else {
>>> *zc = false;
>>> }
>>>
>>> + rxm->full_len -= padding_length(ctx, tls_ctx, skb);
>>> + rxm->offset += prot->prepend_size;
>>> + rxm->full_len -= prot->overhead_size;
>>> + tls_advance_record_sn(sk, &tls_ctx->rx, version);
>>> + ctx->decrypted = true;
>>> + ctx->saved_data_ready(sk);
>>> +
>>> return err;
>>> }
>>
>> This breaks the tls.control_msg test:
>>
>> [ RUN ] tls.control_msg
>> tls.c:764:tls.control_msg:Expected memcmp(buf, test_str, send_len)
>> (18446744073709551614) == 0 (0)
>> tls.c:777:tls.control_msg:Expected memcmp(buf, test_str, send_len)
>> (18446744073709551614) == 0 (0)
>> tls.control_msg: Test failed at step #8
>>
>> So either control message handling needs to only call decrypt_skb_update
>> once, or we need a new flag or something to handle the device case
>
> I prefer to remove variable 'decrypted' in context.
> This is no longer required as we already have an rx_list in context for storing decrypted records.
> So for any record which we decrypted but did not return to user space
> (e.g. for the case when user used recv() and it lead to decryption of non-data record), we should
> it in rx_list.
>
IMO this is inappropriate here, because packets decrypted by tls_device
are ready to be received, and there is no reason to bounce them through
the rx_list.
^ permalink raw reply
* Re: [PATCH net 4/4] tls: Fix tls_device receive
From: Boris Pismenny @ 2019-02-27 15:26 UTC (permalink / raw)
To: Dave Watson
Cc: Aviad Yehezkel, john.fastabend@gmail.com, daniel@iogearbox.net,
vakul.garg@nxp.com, netdev@vger.kernel.org, Eran Ben Elisha
In-Reply-To: <20190226203437.c7tsjfb5ri35nn6y@iphone-a056f37cfbb1.dhcp.thefacebook.com>
On 2/26/2019 10:34 PM, Dave Watson wrote:
> On 02/26/19 02:12 PM, Boris Pismenny wrote:
>> Currently, the receive function fails to handle records already
>> decrypted by the device due to the commit mentioned below.
>>
>> This commit advances the TLS record sequence number and prepares the context
>> to handle the next record.
>>
>> Fixes: fedf201e1296 ("net: tls: Refactor control message handling on recv")
>> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
>> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
>> ---
>> net/tls/tls_sw.c | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
>> index f515cd7e984e..85da10182d8d 100644
>> --- a/net/tls/tls_sw.c
>> +++ b/net/tls/tls_sw.c
>> @@ -1481,18 +1481,17 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
>>
>> return err;
>> }
>> -
>> - rxm->full_len -= padding_length(ctx, tls_ctx, skb);
>> -
>> - rxm->offset += prot->prepend_size;
>> - rxm->full_len -= prot->overhead_size;
>> - tls_advance_record_sn(sk, &tls_ctx->rx, version);
>> - ctx->decrypted = true;
>> - ctx->saved_data_ready(sk);
>> } else {
>> *zc = false;
>> }
>>
>> + rxm->full_len -= padding_length(ctx, tls_ctx, skb);
>> + rxm->offset += prot->prepend_size;
>> + rxm->full_len -= prot->overhead_size;
>> + tls_advance_record_sn(sk, &tls_ctx->rx, version);
>> + ctx->decrypted = true;
>> + ctx->saved_data_ready(sk);
>> +
>> return err;
>> }
>
> This breaks the tls.control_msg test:
>
> [ RUN ] tls.control_msg
> tls.c:764:tls.control_msg:Expected memcmp(buf, test_str, send_len) (18446744073709551614) == 0 (0)
> tls.c:777:tls.control_msg:Expected memcmp(buf, test_str, send_len) (18446744073709551614) == 0 (0)
> tls.control_msg: Test failed at step #8
>
> So either control message handling needs to only call
> decrypt_skb_update once, or we need a new flag or something to handle
> the device case
>
Thanks for raising this, I'm not used to the kselftests yet.
I've refactored the code here to get this working.
Will send V2 soon.
^ permalink raw reply
* [PATCH v2 net 2/4] tls: Fix write space handling
From: Boris Pismenny @ 2019-02-27 15:38 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190227153806.17080-1-borisp@mellanox.com>
TLS device cannot use the sw context. This patch returns the original
tls device write space handler and moves the sw/device specific portions
to the relevant files.
Also, we remove the write_space call for the tls_sw flow, because it
handles partial records in its delayed tx work handler.
Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
---
include/net/tls.h | 3 +++
net/tls/tls_device.c | 17 +++++++++++++++++
net/tls/tls_main.c | 15 ++++++---------
net/tls/tls_sw.c | 13 +++++++++++++
4 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index a528a082da73..a5a938583295 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -519,6 +519,9 @@ static inline bool tls_sw_has_ctx_tx(const struct sock *sk)
return !!tls_sw_ctx_tx(ctx);
}
+void tls_sw_write_space(struct sock *sk, struct tls_context *ctx);
+void tls_device_write_space(struct sock *sk, struct tls_context *ctx);
+
static inline struct tls_offload_context_rx *
tls_offload_ctx_rx(const struct tls_context *tls_ctx)
{
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 3e5e8e021a87..4a1da837a733 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -546,6 +546,23 @@ static int tls_device_push_pending_record(struct sock *sk, int flags)
return tls_push_data(sk, &msg_iter, 0, flags, TLS_RECORD_TYPE_DATA);
}
+void tls_device_write_space(struct sock *sk, struct tls_context *ctx)
+{
+ int rc = 0;
+
+ if (!sk->sk_write_pending && tls_is_partially_sent_record(ctx)) {
+ gfp_t sk_allocation = sk->sk_allocation;
+
+ sk->sk_allocation = GFP_ATOMIC;
+ rc = tls_push_partial_record(sk, ctx,
+ MSG_DONTWAIT | MSG_NOSIGNAL);
+ sk->sk_allocation = sk_allocation;
+ }
+
+ if (!rc)
+ ctx->sk_write_space(sk);
+}
+
void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 7e05af75536d..17e8667917aa 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -212,7 +212,6 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
static void tls_write_space(struct sock *sk)
{
struct tls_context *ctx = tls_get_ctx(sk);
- struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
/* If in_tcp_sendpages call lower protocol write space handler
* to ensure we wake up any waiting operations there. For example
@@ -223,14 +222,12 @@ static void tls_write_space(struct sock *sk)
return;
}
- /* Schedule the transmission if tx list is ready */
- if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
- /* Schedule the transmission */
- if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx->tx_bitmask))
- schedule_delayed_work(&tx_ctx->tx_work.work, 0);
- }
-
- ctx->sk_write_space(sk);
+#ifdef CONFIG_TLS_DEVICE
+ if (ctx->tx_conf == TLS_HW)
+ tls_device_write_space(sk, ctx);
+ else
+#endif
+ tls_sw_write_space(sk, ctx);
}
static void tls_ctx_free(struct tls_context *ctx)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 1cc830582fa8..917caacd4d31 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2126,6 +2126,19 @@ static void tx_work_handler(struct work_struct *work)
release_sock(sk);
}
+void tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
+{
+ struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
+
+ /* Schedule the transmission if tx list is ready */
+ if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
+ /* Schedule the transmission */
+ if (!test_and_set_bit(BIT_TX_SCHEDULED,
+ &tx_ctx->tx_bitmask))
+ schedule_delayed_work(&tx_ctx->tx_work.work, 0);
+ }
+}
+
int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
--
2.12.2
^ permalink raw reply related
* [PATCH v2 net 1/4] tls: Fix tls_device handling of partial records
From: Boris Pismenny @ 2019-02-27 15:38 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190227153806.17080-1-borisp@mellanox.com>
Cleanup the handling of partial records while fixing a bug where the
tls_push_pending_closed_record function is using the software tls
context instead of the hardware context.
The bug resulted in the following crash:
[ 88.791229] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 88.793271] #PF error: [normal kernel read fault]
[ 88.794449] PGD 800000022a426067 P4D 800000022a426067 PUD 22a156067 PMD 0
[ 88.795958] Oops: 0000 [#1] SMP PTI
[ 88.796884] CPU: 2 PID: 4973 Comm: openssl Not tainted 5.0.0-rc4+ #3
[ 88.798314] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[ 88.800067] RIP: 0010:tls_tx_records+0xef/0x1d0 [tls]
[ 88.801256] Code: 00 02 48 89 43 08 e8 a0 0b 96 d9 48 89 df e8 48 dd
4d d9 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39
c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00
[ 88.805179] RSP: 0018:ffffbd888186fca8 EFLAGS: 00010213
[ 88.806458] RAX: ffff9af1ed657c98 RBX: ffff9af1e88a1980 RCX: 0000000000000000
[ 88.808050] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9af1e88a1980
[ 88.809724] RBP: ffff9af1e88a1980 R08: 0000000000000017 R09: ffff9af1ebeeb700
[ 88.811294] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 88.812917] R13: ffff9af1e88a1980 R14: ffff9af1ec13f800 R15: 0000000000000000
[ 88.814506] FS: 00007fcad2240740(0000) GS:ffff9af1f7880000(0000) knlGS:0000000000000000
[ 88.816337] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 88.817717] CR2: 0000000000000000 CR3: 0000000228b3e000 CR4: 00000000001406e0
[ 88.819328] Call Trace:
[ 88.820123] tls_push_data+0x628/0x6a0 [tls]
[ 88.821283] ? remove_wait_queue+0x20/0x60
[ 88.822383] ? n_tty_read+0x683/0x910
[ 88.823363] tls_device_sendmsg+0x53/0xa0 [tls]
[ 88.824505] sock_sendmsg+0x36/0x50
[ 88.825492] sock_write_iter+0x87/0x100
[ 88.826521] __vfs_write+0x127/0x1b0
[ 88.827499] vfs_write+0xad/0x1b0
[ 88.828454] ksys_write+0x52/0xc0
[ 88.829378] do_syscall_64+0x5b/0x180
[ 88.830369] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 88.831603] RIP: 0033:0x7fcad1451680
[ 1248.470626] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 1248.472564] #PF error: [normal kernel read fault]
[ 1248.473790] PGD 0 P4D 0
[ 1248.474642] Oops: 0000 [#1] SMP PTI
[ 1248.475651] CPU: 3 PID: 7197 Comm: openssl Tainted: G OE 5.0.0-rc4+ #3
[ 1248.477426] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[ 1248.479310] RIP: 0010:tls_tx_records+0x110/0x1f0 [tls]
[ 1248.480644] Code: 00 02 48 89 43 08 e8 4f cb 63 d7 48 89 df e8 f7 9c
1b d7 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39
c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00
[ 1248.484825] RSP: 0018:ffffaa0a41543c08 EFLAGS: 00010213
[ 1248.486154] RAX: ffff955a2755dc98 RBX: ffff955a36031980 RCX: 0000000000000006
[ 1248.487855] RDX: 0000000000000000 RSI: 000000000000002b RDI: 0000000000000286
[ 1248.489524] RBP: ffff955a36031980 R08: 0000000000000000 R09: 00000000000002b1
[ 1248.491394] R10: 0000000000000003 R11: 00000000ad55ad55 R12: 0000000000000000
[ 1248.493162] R13: 0000000000000000 R14: ffff955a2abe6c00 R15: 0000000000000000
[ 1248.494923] FS: 0000000000000000(0000) GS:ffff955a378c0000(0000) knlGS:0000000000000000
[ 1248.496847] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1248.498357] CR2: 0000000000000000 CR3: 000000020c40e000 CR4: 00000000001406e0
[ 1248.500136] Call Trace:
[ 1248.500998] ? tcp_check_oom+0xd0/0xd0
[ 1248.502106] tls_sk_proto_close+0x127/0x1e0 [tls]
[ 1248.503411] inet_release+0x3c/0x60
[ 1248.504530] __sock_release+0x3d/0xb0
[ 1248.505611] sock_close+0x11/0x20
[ 1248.506612] __fput+0xb4/0x220
[ 1248.507559] task_work_run+0x88/0xa0
[ 1248.508617] do_exit+0x2cb/0xbc0
[ 1248.509597] ? core_sys_select+0x17a/0x280
[ 1248.510740] do_group_exit+0x39/0xb0
[ 1248.511789] get_signal+0x1d0/0x630
[ 1248.512823] do_signal+0x36/0x620
[ 1248.513822] exit_to_usermode_loop+0x5c/0xc6
[ 1248.515003] do_syscall_64+0x157/0x180
[ 1248.516094] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 1248.517456] RIP: 0033:0x7fb398bd3f53
[ 1248.518537] Code: Bad RIP value.
Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
---
include/net/tls.h | 20 ++++----------------
net/tls/tls_device.c | 9 +++++----
net/tls/tls_main.c | 13 -------------
3 files changed, 9 insertions(+), 33 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index 9f4117ae2297..a528a082da73 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -199,10 +199,6 @@ struct tls_offload_context_tx {
(ALIGN(sizeof(struct tls_offload_context_tx), sizeof(void *)) + \
TLS_DRIVER_STATE_SIZE)
-enum {
- TLS_PENDING_CLOSED_RECORD
-};
-
struct cipher_context {
char *iv;
char *rec_seq;
@@ -335,17 +331,14 @@ int tls_push_sg(struct sock *sk, struct tls_context *ctx,
int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
int flags);
-int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx,
- int flags, long *timeo);
-
static inline struct tls_msg *tls_msg(struct sk_buff *skb)
{
return (struct tls_msg *)strp_msg(skb);
}
-static inline bool tls_is_pending_closed_record(struct tls_context *ctx)
+static inline bool tls_is_partially_sent_record(struct tls_context *ctx)
{
- return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
+ return !!ctx->partially_sent_record;
}
static inline int tls_complete_pending_work(struct sock *sk,
@@ -357,17 +350,12 @@ static inline int tls_complete_pending_work(struct sock *sk,
if (unlikely(sk->sk_write_pending))
rc = wait_on_pending_writer(sk, timeo);
- if (!rc && tls_is_pending_closed_record(ctx))
- rc = tls_push_pending_closed_record(sk, ctx, flags, timeo);
+ if (!rc && tls_is_partially_sent_record(ctx))
+ rc = tls_push_partial_record(sk, ctx, flags);
return rc;
}
-static inline bool tls_is_partially_sent_record(struct tls_context *ctx)
-{
- return !!ctx->partially_sent_record;
-}
-
static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx)
{
return tls_ctx->pending_open_record_frags;
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index a5c17c47d08a..3e5e8e021a87 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -271,7 +271,6 @@ static int tls_push_record(struct sock *sk,
list_add_tail(&record->list, &offload_ctx->records_list);
spin_unlock_irq(&offload_ctx->lock);
offload_ctx->open_record = NULL;
- set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
tls_advance_record_sn(sk, &ctx->tx, ctx->crypto_send.info.version);
for (i = 0; i < record->num_frags; i++) {
@@ -368,9 +367,11 @@ static int tls_push_data(struct sock *sk,
return -sk->sk_err;
timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
- rc = tls_complete_pending_work(sk, tls_ctx, flags, &timeo);
- if (rc < 0)
- return rc;
+ if (tls_is_partially_sent_record(tls_ctx)) {
+ rc = tls_push_partial_record(sk, tls_ctx, flags);
+ if (rc < 0)
+ return rc;
+ }
pfrag = sk_page_frag(sk);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index caff15b2f9b2..7e05af75536d 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -209,19 +209,6 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
return tls_push_sg(sk, ctx, sg, offset, flags);
}
-int tls_push_pending_closed_record(struct sock *sk,
- struct tls_context *tls_ctx,
- int flags, long *timeo)
-{
- struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
-
- if (tls_is_partially_sent_record(tls_ctx) ||
- !list_empty(&ctx->tx_list))
- return tls_tx_records(sk, flags);
- else
- return tls_ctx->push_pending_record(sk, flags);
-}
-
static void tls_write_space(struct sock *sk)
{
struct tls_context *ctx = tls_get_ctx(sk);
--
2.12.2
^ permalink raw reply related
* [PATCH v2 net 4/4] tls: Fix tls_device receive
From: Boris Pismenny @ 2019-02-27 15:38 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190227153806.17080-1-borisp@mellanox.com>
Currently, the receive function fails to handle records already
decrypted by the device due to the commit mentioned below.
This commit advances the TLS record sequence number and prepares the context
to handle the next record.
Fixes: fedf201e1296 ("net: tls: Refactor control message handling on recv")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
---
net/tls/tls_sw.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 68cd026fa57c..425351ac2a9b 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1467,23 +1467,26 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
struct strp_msg *rxm = strp_msg(skb);
int err = 0;
+ if (!ctx->decrypted) {
#ifdef CONFIG_TLS_DEVICE
- err = tls_device_decrypted(sk, skb);
- if (err < 0)
- return err;
+ err = tls_device_decrypted(sk, skb);
+ if (err < 0)
+ return err;
#endif
- if (!ctx->decrypted) {
- err = decrypt_internal(sk, skb, dest, NULL, chunk, zc, async);
- if (err < 0) {
- if (err == -EINPROGRESS)
- tls_advance_record_sn(sk, &tls_ctx->rx,
- version);
+ /* Still not decrypted after tls_device */
+ if (!ctx->decrypted) {
+ err = decrypt_internal(sk, skb, dest, NULL, chunk, zc,
+ async);
+ if (err < 0) {
+ if (err == -EINPROGRESS)
+ tls_advance_record_sn(sk, &tls_ctx->rx,
+ version);
- return err;
+ return err;
+ }
}
rxm->full_len -= padding_length(ctx, tls_ctx, skb);
-
rxm->offset += prot->prepend_size;
rxm->full_len -= prot->overhead_size;
tls_advance_record_sn(sk, &tls_ctx->rx, version);
--
2.12.2
^ permalink raw reply related
* [PATCH v2 net 0/4] tls: Fix issues in tls_device
From: Boris Pismenny @ 2019-02-27 15:38 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
This series fixes issues encountered in tls_device code paths,
which were introduced recently.
Additionally, this series includes a fix for tls software only receive flow,
which causes corruption of payload received by user space applications.
This series was tested using the OpenSSL integration of KTLS -
https://github.com/mellan
Boris Pismenny (3):
tls: Fix tls_device handling of partial records
tls: Fix write space handling
tls: Fix tls_device receive
Eran Ben Elisha (1):
tls: Fix mixing between async capable and async
include/net/tls.h | 23 +++++++----------------
net/tls/tls_device.c | 26 ++++++++++++++++++++++----
net/tls/tls_main.c | 28 ++++++---------------------
net/tls/tls_sw.c | 53 +++++++++++++++++++++++++++++++++++-----------------
4 files changed, 71 insertions(+), 59 deletions(-)
--
2.12.2
^ permalink raw reply
* [PATCH v2 net 3/4] tls: Fix mixing between async capable and async
From: Boris Pismenny @ 2019-02-27 15:38 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190227153806.17080-1-borisp@mellanox.com>
From: Eran Ben Elisha <eranbe@mellanox.com>
Today, tls_sw_recvmsg is capable of using asynchronous mode to handle
application data TLS records. Moreover, it assumes that if the cipher
can be handled asynchronously, then all packets will be processed
asynchronously.
However, this assumption is not always true. Specifically, for AES-GCM
in TLS1.2, it causes data corruption, and breaks user applications.
This patch fixes this problem by separating the async capability from
the decryption operation result.
Fixes: c0ab4732d4c6 ("net/tls: Do not use async crypto for non-data records")
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Reviewed-by: Boris Pismenny <borisp@mellanox.com>
---
net/tls/tls_sw.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 917caacd4d31..68cd026fa57c 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1693,7 +1693,8 @@ int tls_sw_recvmsg(struct sock *sk,
bool zc = false;
int to_decrypt;
int chunk = 0;
- bool async;
+ bool async_capable;
+ bool async = false;
skb = tls_wait_data(sk, psock, flags, timeo, &err);
if (!skb) {
@@ -1727,21 +1728,23 @@ int tls_sw_recvmsg(struct sock *sk,
/* Do not use async mode if record is non-data */
if (ctx->control == TLS_RECORD_TYPE_DATA)
- async = ctx->async_capable;
+ async_capable = ctx->async_capable;
else
- async = false;
+ async_capable = false;
err = decrypt_skb_update(sk, skb, &msg->msg_iter,
- &chunk, &zc, async);
+ &chunk, &zc, async_capable);
if (err < 0 && err != -EINPROGRESS) {
tls_err_abort(sk, EBADMSG);
goto recv_end;
}
- if (err == -EINPROGRESS)
+ if (err == -EINPROGRESS) {
+ async = true;
num_async++;
- else if (prot->version == TLS_1_3_VERSION)
+ } else if (prot->version == TLS_1_3_VERSION) {
tlm->control = ctx->control;
+ }
/* If the type of records being processed is not known yet,
* set it to record type just dequeued. If it is already known,
--
2.12.2
^ permalink raw reply related
* Re: [PATCH v2 3/6] sched/cpufreq: Annotate cpufreq_update_util_data pointer with __rcu
From: Joel Fernandes @ 2019-02-27 15:42 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Peter Zijlstra, Quentin Perret, rcu,
Song Liu, Steven Rostedt, Vincent Guittot, xdp-newbies,
Yonghong Song
In-Reply-To: <20190225211026.GX4072@linux.ibm.com>
On Mon, Feb 25, 2019 at 01:10:26PM -0800, Paul E. McKenney wrote:
> On Sat, Feb 23, 2019 at 01:34:31AM -0500, Joel Fernandes (Google) wrote:
> > Recently I added an RCU annotation check to rcu_assign_pointer(). All
> > pointers assigned to RCU protected data are to be annotated with __rcu
> > inorder to be able to use rcu_assign_pointer() similar to checks in
> > other RCU APIs.
> >
> > This resulted in a sparse error: kernel//sched/cpufreq.c:41:9: sparse:
> > error: incompatible types in comparison expression (different address
> > spaces)
> >
> > Fix this by annotating cpufreq_update_util_data pointer with __rcu. This
> > will also help sparse catch any future RCU misuage bugs.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
> From an RCU perspective:
>
> Reviewed-by: Paul E. McKenney <paulmck@linux.ibm.com>
Thanks a lot Paul.
Peter, Rafael, Steve,
Are you Ok with the patches 3-6? It will be nice to quiet down sparse.
thanks,
- Joel
> > ---
> > kernel/sched/cpufreq.c | 2 +-
> > kernel/sched/sched.h | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c
> > index 22bd8980f32f..e316ee7bb2e5 100644
> > --- a/kernel/sched/cpufreq.c
> > +++ b/kernel/sched/cpufreq.c
> > @@ -7,7 +7,7 @@
> > */
> > #include "sched.h"
> >
> > -DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
> > +DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
> >
> > /**
> > * cpufreq_add_update_util_hook - Populate the CPU's update_util_data pointer.
> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > index d04530bf251f..2ab545d40381 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -2166,7 +2166,7 @@ static inline u64 irq_time_read(int cpu)
> > #endif /* CONFIG_IRQ_TIME_ACCOUNTING */
> >
> > #ifdef CONFIG_CPU_FREQ
> > -DECLARE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
> > +DECLARE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
> >
> > /**
> > * cpufreq_update_util - Take a note about CPU utilization changes.
> > --
> > 2.21.0.rc0.258.g878e2cd30e-goog
> >
>
^ permalink raw reply
* Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
From: Eric Dumazet @ 2019-02-27 15:53 UTC (permalink / raw)
To: Sheng Lan, Eric Dumazet, Stephen Hemminger
Cc: davem, netdev, netem, xuhanbing, zhengshaoyu, jiqin.ji,
liuzhiqiang26, yuehaibing
In-Reply-To: <dd75923d-a079-7db0-6903-a31fff062d26@huawei.com>
On 02/27/2019 03:26 AM, Sheng Lan wrote:
>
> I traced again and found that the skb was not sent, master skb was still in write queue,
> because the function tcp_transmit_skb() returns 1 (NET_XMIT_DROP), thus it can be retransmit.
> I found the error value NET_XMIT_DROP returns from netem_enqueue(), when the length of qdisc queue
> is greater than queue limit value.
>
> In netem_enqueue() the skb is cloned before returning the NET_XMIT_DROP error value,
> thus the master skb is still in write queue and be cloned in netem_enqueue(). This may cause the master
> skb be retransmit and fragmented again while it is cloned.
>
> I think there are potential risks that tso_fragment() will get a cloned skb if skb is cloned by lower layer.
> I try to fix it by moving returning error value statment to the front of the skb_clone() in netem_enqueue(), and it works.
> And netem_enqueue() constructs corrupt packets statment returns NET_XMIT_DROP too. To fix this completely should I move the
> constructing corrupt statment to the front of the skb_clone() ?
> Please correct me if I am wrong, and I need your advice.
>
Hi
Choices are either :
1) netem sends a proper return value if a packet has been queued.
2) TCP (and probably other protocols) no longer trust lower stack
and always move the master skb in rtx queue,
even if the transmit of the (first) clone failed.
I prefer 1), since netem is not used in the fast path, generally...
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 75046ec7214449c631c38eaab5e4a51644cfa0e5..f6ea2d44dffe328a2fd1a468e209aac0bfaccd49 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -447,6 +447,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
int nb = 0;
int count = 1;
int rc = NET_XMIT_SUCCESS;
+ int rc_drop = NET_XMIT_DROP;
/* Do not fool qdisc_drop_all() */
skb->prev = NULL;
@@ -486,6 +487,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
q->duplicate = 0;
rootq->enqueue(skb2, rootq, to_free);
q->duplicate = dupsave;
+ rc_drop = NET_XMIT_SUCCESS;
}
/*
@@ -498,7 +500,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
if (skb_is_gso(skb)) {
segs = netem_segment(skb, sch, to_free);
if (!segs)
- return NET_XMIT_DROP;
+ return rc_drop;
} else {
segs = skb;
}
@@ -521,9 +523,10 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
1<<(prandom_u32() % 8);
}
- if (unlikely(sch->q.qlen >= sch->limit))
- return qdisc_drop_all(skb, sch, to_free);
-
+ if (unlikely(sch->q.qlen >= sch->limit)) {
+ qdisc_drop_all(skb, sch, to_free);
+ return rc_drop;
+ }
qdisc_qstats_backlog_inc(sch, skb);
cb = netem_skb_cb(skb);
^ permalink raw reply related
* [PATCH bpf-next] bpf: add missing entries to bpf_helpers.h
From: Willem de Bruijn @ 2019-02-27 16:08 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kafai, sdf, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
This header defines the BPF functions enumerated in uapi/linux.bpf.h
in a callable format. Expand to include all registered functions.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
changes pre->v1:
- add brief commit message
(not copying Acks as they do not necessarily agree with that)
- change return type from ull to uint for
bpf_get_cgroup_classid, bpf_get_route_realm, bpf_get_hash_recalc
- change size arg from int to __u32 for bpf_probe_read_str
---
tools/testing/selftests/bpf/bpf_helpers.h | 30 +++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index d9999f1ed1d2a..026bea831e039 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -232,6 +232,36 @@ static int (*bpf_skb_change_head)(void *, int len, int flags) =
(void *) BPF_FUNC_skb_change_head;
static int (*bpf_skb_pull_data)(void *, int len) =
(void *) BPF_FUNC_skb_pull_data;
+static unsigned int (*bpf_get_cgroup_classid)(void *ctx) =
+ (void *) BPF_FUNC_get_cgroup_classid;
+static unsigned int (*bpf_get_route_realm)(void *ctx) =
+ (void *) BPF_FUNC_get_route_realm;
+static int (*bpf_skb_change_proto)(void *ctx, __be16 proto, __u64 flags) =
+ (void *) BPF_FUNC_skb_change_proto;
+static int (*bpf_skb_change_type)(void *ctx, __u32 type) =
+ (void *) BPF_FUNC_skb_change_type;
+static unsigned int (*bpf_get_hash_recalc)(void *ctx) =
+ (void *) BPF_FUNC_get_hash_recalc;
+static unsigned long long (*bpf_get_current_task)(void *ctx) =
+ (void *) BPF_FUNC_get_current_task;
+static int (*bpf_skb_change_tail)(void *ctx, __u32 len, __u64 flags) =
+ (void *) BPF_FUNC_skb_change_tail;
+static long long (*bpf_csum_update)(void *ctx, __u32 csum) =
+ (void *) BPF_FUNC_csum_update;
+static void (*bpf_set_hash_invalid)(void *ctx) =
+ (void *) BPF_FUNC_set_hash_invalid;
+static int (*bpf_get_numa_node_id)(void) =
+ (void *) BPF_FUNC_get_numa_node_id;
+static int (*bpf_probe_read_str)(void *ctx, __u32 size,
+ const void *unsafe_ptr) =
+ (void *) BPF_FUNC_probe_read_str;
+static unsigned int (*bpf_get_socket_uid)(void *ctx) =
+ (void *) BPF_FUNC_get_socket_uid;
+static unsigned int (*bpf_set_hash)(void *ctx, __u32 hash) =
+ (void *) BPF_FUNC_set_hash;
+static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
+ unsigned long long flags) =
+ (void *) BPF_FUNC_skb_adjust_room;
/* Scan the ARCH passed in from ARCH env variable (see Makefile) */
#if defined(__TARGET_ARCH_x86)
--
2.21.0.rc2.261.ga7da99ff1b-goog
^ permalink raw reply related
* Re: [PATCH net-next] net: sched: pie: fix 64-bit division
From: Leslie Monis @ 2019-02-27 16:12 UTC (permalink / raw)
To: David Laight; +Cc: netdev
In-Reply-To: <44c1c447befd45e49a06b1e3d3f5f7a6@AcuMS.aculab.com>
On Wed, Feb 27, 2019 at 10:11:14AM +0000, David Laight wrote:
> From: Leslie Monis
> > Sent: 27 February 2019 01:00
> > Use div_u64() to resolve build failures on 32-bit platforms.
> >
> > Fixes: 3f7ae5f3dc52 ("net: sched: pie: add more cases to auto-tune alpha and beta")
> > Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
> > ---
> > net/sched/sch_pie.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
> > index 4c0670b6aec1..f93cfe034c72 100644
> > --- a/net/sched/sch_pie.c
> > +++ b/net/sched/sch_pie.c
> > @@ -429,7 +429,7 @@ static void calculate_probability(struct Qdisc *sch)
> > */
> >
> > if (qdelay == 0 && qdelay_old == 0 && update_prob)
> > - q->vars.prob = (q->vars.prob * 98) / 100;
> > + q->vars.prob = 98 * div_u64(q->vars.prob, 100);
>
> This has significantly different rounding after the change.
> The result for small values is very different.
> The alterative:
> q->vars.prob -= div_u64(q->vars.prob, 50);
> is much nearer to the original - but never goes to zero.
>
> If the 98% decay factor isn't critical then you could remove
> 1/64th or 1/32nd + 1/16th to avoid the slow division.
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
Hi David,
You're right, the change does make the result for small
values different. I made it anyway as the probability
value is scaled by u64. It is safe to say that q->vars.prob
holds relatively large values (in its scaled form) in all
cases where it isn't 0.
But, I think we can avoid the slow division here. RFC 8033
does say that using (1 - 1/64) should be sufficient. This
will give us:
q-vars.prob -= q->vars.prob >> 6;
which I feel would be much better. What do you reckon?
Thanks a lot for the feedback.
Cheers,
Leslie
^ permalink raw reply
* Re: [PATCH net v3 2/2] selftests: pmtu: add explicit tests for PMTU exceptions cleanup
From: David Ahern @ 2019-02-27 16:18 UTC (permalink / raw)
To: Paolo Abeni, netdev; +Cc: David S. Miller, Stefano Brivio, Sabrina Dubroca
In-Reply-To: <acec2def2f7f6d254c539c9dd5fb2001f1845524.1551106591.git.pabeni@redhat.com>
On 2/25/19 8:08 AM, Paolo Abeni wrote:
> Add a couple of new tests, explicitly checking that the kernel
> timely releases PMTU exceptions on related device removal.
> This is mostly a regression test vs the issue fixed by
> commit f5b51fe804ec ("ipv6: route: purge exception on removal")
>
> Only 2 new test cases have been added, instead of extending all
> the existing ones, because the reproducer requires executing
> several commands and would slow down too much the tests otherwise.
>
> v2 -> v3:
> - more cleanup, still from Stefano
>
> v1 -> v2:
> - several script cleanups, as suggested by Stefano
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> tools/testing/selftests/net/pmtu.sh | 68 ++++++++++++++++++++++++++++-
> 1 file changed, 67 insertions(+), 1 deletion(-)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
Thanks for adding the tests.
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the rdma tree
From: Doug Ledford @ 2019-02-27 16:24 UTC (permalink / raw)
To: Stephen Rothwell, David Miller, Networking, Jason Gunthorpe
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Shamir Rabinovitch, Jakub Kicinski
In-Reply-To: <20190227112522.2f32b5d7@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]
On Wed, 2019-02-27 at 11:25 +1100, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in:
>
> drivers/infiniband/hw/mlx4/Kconfig
>
> between commit:
>
> 6fa8f1afd337 ("IB/{core,uverbs}: Move ib_umem_xxx functions from ib_core to ib_uverbs")
>
> from the rdma tree and commit:
>
> f4b6bcc7002f ("net: devlink: turn devlink into a built-in")
>
> from the net-next tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
Thanks Stephen, we'll add it to the (largish this release) list of
conflicts to bring to Linus' attention.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH iproute2-next] rdma: Add the prefix for driver attributes
From: David Ahern @ 2019-02-27 16:26 UTC (permalink / raw)
To: Leon Romanovsky, Lijun Ou, Steve Wise
Cc: Leon Romanovsky, netdev, RDMA mailing list, Stephen Hemminger
In-Reply-To: <20190227064151.25445-1-leon@kernel.org>
On 2/26/19 11:41 PM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> There is a need to distinguish between driver vs. general exposed
> attributes. The most common use case is to expose some internal
> garbage under extremely common and sexy name, e.g. pi, ci e.t.c
>
> In order to achieve that, we will add "drv_" prefix to all strings
> which were received through RDMA_NLDEV_ATTR_DRIVER_* attributes.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>a
> ---
> rdma/utils.c | 34 ++++++++++++++++++++++------------
> 1 file changed, 22 insertions(+), 12 deletions(-)
>
applied to iproute2-next. Thanks
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox