* [patch net-next v2 1/3] net: sched: change name of zombie chain to "held_by_acts_only"
From: Jiri Pirko @ 2018-08-01 10:36 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, mlxsw
In-Reply-To: <20180801103657.10532-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
As mentioned by Cong and Jakub during the review process, it is a bit
odd to sometimes (act flow) create a new chain which would be
immediately a "zombie". So just rename it to "held_by_acts_only".
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index e20aad1987b8..2f78341f2888 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -272,11 +272,10 @@ static void tcf_chain_release_by_act(struct tcf_chain *chain)
--chain->action_refcnt;
}
-static bool tcf_chain_is_zombie(struct tcf_chain *chain)
+static bool tcf_chain_held_by_acts_only(struct tcf_chain *chain)
{
/* In case all the references are action references, this
- * chain is a zombie and should not be listed in the chain
- * dump list.
+ * chain should not be shown to the user.
*/
return chain->refcnt == chain->action_refcnt;
}
@@ -1838,10 +1837,9 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
chain = tcf_chain_lookup(block, chain_index);
if (n->nlmsg_type == RTM_NEWCHAIN) {
if (chain) {
- if (tcf_chain_is_zombie(chain)) {
+ if (tcf_chain_held_by_acts_only(chain)) {
/* The chain exists only because there is
- * some action referencing it, meaning it
- * is a zombie.
+ * some action referencing it.
*/
tcf_chain_hold(chain);
} else {
@@ -1860,7 +1858,7 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
}
}
} else {
- if (!chain || tcf_chain_is_zombie(chain)) {
+ if (!chain || tcf_chain_held_by_acts_only(chain)) {
NL_SET_ERR_MSG(extack, "Cannot find specified filter chain");
return -EINVAL;
}
@@ -1988,7 +1986,7 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
index++;
continue;
}
- if (tcf_chain_is_zombie(chain))
+ if (tcf_chain_held_by_acts_only(chain))
continue;
err = tc_chain_fill_node(chain, net, skb, block,
NETLINK_CB(cb->skb).portid,
--
2.14.4
^ permalink raw reply related
* [patch net-next v2 0/3] net: sched: couple of adjustments/fixes
From: Jiri Pirko @ 2018-08-01 10:36 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Jiri Pirko (3):
net: sched: change name of zombie chain to "held_by_acts_only"
net: sched: fix notifications for action-held chains
net: sched: make tcf_chain_{get,put}() static
include/net/pkt_cls.h | 3 --
net/sched/cls_api.c | 113 +++++++++++++++++++++++++++-----------------------
2 files changed, 62 insertions(+), 54 deletions(-)
--
2.14.4
^ permalink raw reply
* Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation)
From: Florian Westphal @ 2018-08-01 10:35 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Linus Torvalds, Christoph Lameter, Andrey Ryabinin,
Theodore Ts'o, Jan Kara, linux-ext4, Greg Kroah-Hartman,
Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David Miller, NetFilter, coreteam, Network Development,
Gerrit Renker, dccp, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Dave Airlie
In-Reply-To: <CACT4Y+ZkgqDT77dshHg+hBtc9YPW-eZ8wVQA9LTDQ6q_y99oiQ@mail.gmail.com>
Dmitry Vyukov <dvyukov@google.com> wrote:
> Still can't grasp all details.
> There is state that we read without taking ct->ct_general.use ref
> first, namely ct->state and what's used by nf_ct_key_equal.
> So let's say the entry we want to find is in the list, but
> ____nf_conntrack_find finds a wrong entry earlier because all state it
> looks at is random garbage, so it returns the wrong entry to
> __nf_conntrack_find_get.
If an entry can be found, it can't be random garbage.
We never link entries into global table until state has been set up.
> Now (nf_ct_is_dying(ct) || !atomic_inc_not_zero(&ct->ct_general.use))
> check in __nf_conntrack_find_get passes, and it returns NULL to the
> caller (which means entry is not present).
So entry is going away or marked as dead which for us is same as
'not present', we need to allocate a new entry.
> While in reality the entry
> is present, but we were just looking at the wrong one.
We never add tuples that are identical to the global table.
If N cores receive identical packets at same time with no prior state, all
will allocate a new conntrack, but we notice this when we try to insert the
nf_conn entries into the table.
Only one will succeed, other cpus have to cope with this.
(worst case: all raced packets are dropped along with their conntrack
object).
For lookup, we have following scenarios:
1. It doesn't exist -> new allocation needed
2. It exists, not dead, has nonzero refount -> use it
3. It exists, but marked as dying -> new allocation needed
4. It exists but has 0 reference count -> new allocation needed
5. It exists, we get reference, but 2nd nf_ct_key_equal check
fails. We saw a matching 'old incarnation' that just got
re-used on other core. -> retry lookup
> Also I am not sure about order of checks in (nf_ct_is_dying(ct) ||
> !atomic_inc_not_zero(&ct->ct_general.use)), because checking state
> before taking the ref is only a best-effort hint, so it can actually
> be a dying entry when we take a ref.
Yes, it can also become a dying entry after we took the reference.
> So shouldn't it read something like the following?
>
> rcu_read_lock();
> begin:
> h = ____nf_conntrack_find(net, zone, tuple, hash);
> if (h) {
> ct = nf_ct_tuplehash_to_ctrack(h);
> if (!atomic_inc_not_zero(&ct->ct_general.use))
> goto begin;
> if (unlikely(nf_ct_is_dying(ct)) ||
> unlikely(!nf_ct_key_equal(h, tuple, zone, net))) {
> nf_ct_put(ct);
It would be ok to make this change, but dying bit can be set
at any time e.g. because userspace tells kernel to flush the conntrack table.
So refcount is always > 0 when the DYING bit is set.
I don't see why it would be a problem.
nf_conn struct will stay valid until all cpus have dropped references.
The check in lookup function only serves to hide the known-to-go-away entry.
^ permalink raw reply
* 答复: [Patch net-next] net: hns3: fix return value error while hclge_cmd_csq_clean failed
From: tanhuazhong @ 2018-08-01 10:00 UTC (permalink / raw)
To: tanhuazhong, davem@davemloft.net; +Cc: netdev@vger.kernel.org, Linuxarm
In-Reply-To: <1533117208-164535-1-git-send-email-tanhuazhong@huawei.com>
Sorry, please ignore this patch. I will resend it.
-----邮件原件-----
发件人: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] 代表 Huazhong Tan
发送时间: 2018年8月1日 17:53
收件人: davem@davemloft.net
抄送: netdev@vger.kernel.org; Linuxarm <linuxarm@huawei.com>
主题: [Patch net-next] net: hns3: fix return value error while hclge_cmd_csq_clean failed
From: fredalu <fredalu@yeah.net>
While cleaning the command queue, the value of the HEAD register is not in the range of next_to_clean and next_to_use, meaning that this value is invalid. This also means that there is a hardware error and the hardware will trigger a reset soon. At this time we should return an error code instead of 0, and HCLGE_STATE_CMD_DISABLE needs to be set to prevent sending command again.
Fixes: 3ff504908f95 ("net: hns3: fix a dead loop in hclge_cmd_csq_clean")
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>wq
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
index 165c3d5..ac13cb2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
@@ -147,7 +147,12 @@ static int hclge_cmd_csq_clean(struct hclge_hw *hw)
if (!is_valid_csq_clean_head(csq, head)) {
dev_warn(&hdev->pdev->dev, "wrong cmd head (%d, %d-%d)\n", head,
csq->next_to_use, csq->next_to_clean);
- return 0;
+ dev_warn(&hdev->pdev->dev,
+ "Disabling any further commands to IMP firmware\n");
+ set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
+ dev_warn(&hdev->pdev->dev,
+ "IMP firmware watchdog reset soon expected!\n");
+ return -EIO;
}
clean = (head - csq->next_to_clean + csq->desc_num) % csq->desc_num; @@ -267,10 +272,11 @@ int hclge_cmd_send(struct hclge_hw *hw, struct hclge_desc *desc, int num)
/* Clean the command send queue */
handle = hclge_cmd_csq_clean(hw);
- if (handle != num) {
+ if (handle < 0)
+ retval = handle;
+ else if (handle != num)
dev_warn(&hdev->pdev->dev,
"cleaned %d, need to clean %d\n", handle, num);
- }
spin_unlock_bh(&hw->cmq.csq.lock);
--
2.7.4
^ permalink raw reply related
* [Patch net-next V2] net: hns3: fix return value error while hclge_cmd_csq_clean failed
From: Huazhong Tan @ 2018-08-01 10:27 UTC (permalink / raw)
To: davem; +Cc: netdev, linuxarm
While cleaning the command queue, the value of the HEAD register is not
in the range of next_to_clean and next_to_use, meaning that this value
is invalid. This also means that there is a hardware error and the
hardware will trigger a reset soon. At this time we should return an
error code instead of 0, and HCLGE_STATE_CMD_DISABLE needs to be set to
prevent sending command again.
Fixes: 3ff504908f95 ("net: hns3: fix a dead loop in hclge_cmd_csq_clean")
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
index 165c3d5..ac13cb2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
@@ -147,7 +147,12 @@ static int hclge_cmd_csq_clean(struct hclge_hw *hw)
if (!is_valid_csq_clean_head(csq, head)) {
dev_warn(&hdev->pdev->dev, "wrong cmd head (%d, %d-%d)\n", head,
csq->next_to_use, csq->next_to_clean);
- return 0;
+ dev_warn(&hdev->pdev->dev,
+ "Disabling any further commands to IMP firmware\n");
+ set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
+ dev_warn(&hdev->pdev->dev,
+ "IMP firmware watchdog reset soon expected!\n");
+ return -EIO;
}
clean = (head - csq->next_to_clean + csq->desc_num) % csq->desc_num;
@@ -267,10 +272,11 @@ int hclge_cmd_send(struct hclge_hw *hw, struct hclge_desc *desc, int num)
/* Clean the command send queue */
handle = hclge_cmd_csq_clean(hw);
- if (handle != num) {
+ if (handle < 0)
+ retval = handle;
+ else if (handle != num)
dev_warn(&hdev->pdev->dev,
"cleaned %d, need to clean %d\n", handle, num);
- }
spin_unlock_bh(&hw->cmq.csq.lock);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] rxrpc: Fix user ID check in rxrpc_service_prealloc_one
From: David Howells @ 2018-08-01 12:05 UTC (permalink / raw)
To: YueHaibing; +Cc: dhowells, davem, linux-kernel, netdev, linux-afs
In-Reply-To: <20180801094651.7136-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> There just check the user ID isn't already in use, hence should
> compare user_call_ID with xcall->user_call_ID, which is current
> node's user_call_ID.
I've changed "user ID" to "user call ID" to avoid confusion.
David
^ permalink raw reply
* RE: [PATCH 3/3] ptp_qoriq: convert to use module parameters for initialization
From: Y.b. Lu @ 2018-08-01 10:10 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev@vger.kernel.org, Madalin-cristian Bucur, Rob Herring,
Shawn Guo, David S . Miller, devicetree@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20180801061515.krn52iaq2kqdi3wo@localhost>
Hi Richard,
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Wednesday, August 1, 2018 2:15 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; Madalin-cristian Bucur
> <madalin.bucur@nxp.com>; Rob Herring <robh+dt@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; David S . Miller <davem@davemloft.net>;
> devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/3] ptp_qoriq: convert to use module parameters for
> initialization
>
> On Wed, Aug 01, 2018 at 04:36:40AM +0000, Y.b. Lu wrote:
>
> > Could I add a function to calculate a set of default register values
> > to initialize ptp timer when dts method failed to get required
> > properties in driver?
>
> Yes, it would be ideal if the driver can pick correct values automatically.
>
> However, the frequency on the FIPER outputs can't be configured
> automatically, and we don't have an API for the user to choose this.
[Y.b. Lu] Thanks a lot. Just let me send out the v2 patch for your reviewing.
>
> > I think this will be useful. The ptp timer on new platforms (you may
> > see two dts patches in this patchset. Many platforms will be
> > affected.) will work without these dts properties. If user want
> > specific setting, they can set dts properties.
>
> Sure.
>
> Thanks,
> Richard
^ permalink raw reply
* RE: [PATCH net-next] qed: Make some functions static
From: Bolotin, Denis @ 2018-08-01 10:05 UTC (permalink / raw)
To: YueHaibing, davem@davemloft.net, Elior, Ariel,
Dept-Eng Everest Linux L2
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20180731141230.10780-1-yuehaibing@huawei.com>
> From: YueHaibing <yuehaibing@huawei.com>
> Sent: Tuesday, July 31, 2018 5:13 PM
> To: davem@davemloft.net; Elior, Ariel <Ariel.Elior@cavium.com>; Dept-Eng
> Everest Linux L2 <Dept-EngEverestLinuxL2@cavium.com>
> Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org; YueHaibing
> <yuehaibing@huawei.com>
> Subject: [PATCH net-next] qed: Make some functions static
>
> Fixes the following sparse warning:
>
> drivers/net/ethernet/qlogic/qed/qed_cxt.c:1534:6: warning: symbol
> 'qed_cm_init_pf' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_dev.c:233:4: warning: symbol
> 'qed_init_qm_get_num_tcs' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_dev.c:238:5: warning: symbol
> 'qed_init_qm_get_num_vfs' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_dev.c:246:5: warning: symbol
> 'qed_init_qm_get_num_pf_rls' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_dev.c:264:5: warning: symbol
> 'qed_init_qm_get_num_vports' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_dev.c:276:5: warning: symbol
> 'qed_init_qm_get_num_pqs' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_mcp.c:573:5: warning: symbol
> 'qed_mcp_nvm_wr_cmd' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_mcp.c:3012:1: warning: symbol
> '__qed_mcp_resc_lock' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_dcbx.c:870:6: warning: symbol
> 'qed_dcbx_aen' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_debug.c:7841:5: warning: symbol
> 'qed_dbg_nvm_image_length' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_debug.c:7857:5: warning: symbol
> 'qed_dbg_nvm_image' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_sriov.c:675:6: warning: symbol
> '_qed_iov_pf_sanity_check' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_sriov.c:690:6: warning: symbol
> 'qed_iov_pf_sanity_check' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_sriov.c:3982:6: warning: symbol
> 'qed_iov_pf_get_pending_events' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_vf.c:172:5: warning: symbol
> '_qed_vf_pf_release' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_rdma.c:137:5: warning: symbol
> 'qed_rdma_get_sb_id' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_rdma.c:709:5: warning: symbol
> 'qed_rdma_stop' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_ll2.c:161:6: warning: symbol
> 'qed_ll2b_complete_rx_packet' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_roce.c:160:6: warning: symbol
> 'qed_roce_free_cid_pair' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_iwarp.c:380:12: warning: symbol
> 'iwarp_state_names' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_iwarp.c:946:1: warning: symbol
> 'qed_iwarp_parse_private_data' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_iwarp.c:971:1: warning: symbol
> 'qed_iwarp_mpa_reply_arrived' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_iwarp.c:2504:1: warning: symbol
> 'qed_iwarp_ll2_slowpath' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_iwarp.c:2806:6: warning: symbol
> 'qed_iwarp_qp_in_error' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_iwarp.c:2827:6: warning: symbol
> 'qed_iwarp_exception_received' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_iwarp.c:2958:1: warning: symbol
> 'qed_iwarp_connect_complete' was not declared. Should it be static?
> drivers/net/ethernet/qlogic/qed/qed_iscsi.c:876:6: warning: symbol
> 'qed_iscsi_free_connection' was not declared. Should it be static?
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Thanks
Acked-by: Denis Bolotin <Denis.Bolotin@cavium.com>
^ permalink raw reply
* [PATCH] net: fec: check DMA addressing limitations
From: Stefan Agner @ 2018-08-01 11:44 UTC (permalink / raw)
To: fugang.duan, davem; +Cc: krzk, robin.murphy, netdev, linux-kernel, Stefan Agner
Check DMA addressing limitations as suggested by the DMA API
how-to. This does not fix a particular issue seen but is
considered good style.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
drivers/net/ethernet/freescale/fec_main.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index c729665107f5..af0fb200e936 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3146,6 +3146,12 @@ static int fec_enet_init(struct net_device *ndev)
fep->tx_align = 0x3;
#endif
+ /* Check mask of the streaming and coherent API */
+ if (dma_set_mask_and_coherent(&fep->pdev->dev, DMA_BIT_MASK(32))) {
+ dev_warn(&fep->pdev->dev, "No suitable DMA available\n");
+ return -ENODEV;
+ }
+
fec_enet_alloc_queue(ndev);
bd_size = (fep->total_tx_ring_size + fep->total_rx_ring_size) * dsize;
--
2.18.0
^ permalink raw reply related
* Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation)
From: Florian Westphal @ 2018-08-01 11:40 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Florian Westphal, Linus Torvalds, Christoph Lameter,
Andrey Ryabinin, Theodore Ts'o, Jan Kara, linux-ext4,
Greg Kroah-Hartman, Pablo Neira Ayuso, Jozsef Kadlecsik,
David Miller, NetFilter, coreteam, Network Development,
Gerrit Renker, dccp, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Dave Airlie
In-Reply-To: <CACT4Y+aHWpgDZygXv=smWwdVMWfjpedyajuVvvLDGMK-wFD5Lw@mail.gmail.com>
Dmitry Vyukov <dvyukov@google.com> wrote:
> On Wed, Aug 1, 2018 at 12:35 PM, Florian Westphal <fw@strlen.de> wrote:
> > Dmitry Vyukov <dvyukov@google.com> wrote:
> >> Still can't grasp all details.
> >> There is state that we read without taking ct->ct_general.use ref
> >> first, namely ct->state and what's used by nf_ct_key_equal.
> >> So let's say the entry we want to find is in the list, but
> >> ____nf_conntrack_find finds a wrong entry earlier because all state it
> >> looks at is random garbage, so it returns the wrong entry to
> >> __nf_conntrack_find_get.
> >
> > If an entry can be found, it can't be random garbage.
> > We never link entries into global table until state has been set up.
>
> But... we don't hold a reference to the entry. So say it's in the
> table with valid state, now ____nf_conntrack_find discovers it, now
> the entry is removed and reused a dozen of times will all associated
> state reinitialization. And nf_ct_key_equal looks at it concurrently
> and decides if it's the entry we are looking for or now. I think
> unless we hold a ref to the entry, it's state needs to be considered
> random garbage for correctness reasoning.
It checks if it might be the entry we're looking for.
If this was complete random garbage, scheme would not work, as then
we could have entry that isn't in table, has nonzero refcount, but
has its confirmed bit set.
I don't see how that would be possible, any reallocation
makes sure ct->status has CONFIRMED bit clear before setting refcount
to nonzero value.
I think this is the scenario you hint at is:
1. nf_ct_key_equal is true
2. the entry is free'd (or was already free'd)
3. we return NULL to caller due to atomic_inc_not_zero() failure
but i fail to see how thats wrong?
Sure, we could restart lookup but how would that help?
We'd not find the 'candidate' entry again.
We might find entry that has been inserted at this very instant but
newly allocated entries are only inserted into global table until the skb that
created the nf_conn object has made it through the network stack
(postrouting for fowarded, or input for local delivery).
So, the likelyhood of such restart finding another candidate is close to 0,
and won't prevent 'insert race' from happening.
^ permalink raw reply
* [Patch net-next] net: hns3: fix return value error while hclge_cmd_csq_clean failed
From: Huazhong Tan @ 2018-08-01 9:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linuxarm
From: fredalu <fredalu@yeah.net>
While cleaning the command queue, the value of the HEAD register is not
in the range of next_to_clean and next_to_use, meaning that this value
is invalid. This also means that there is a hardware error and the
hardware will trigger a reset soon. At this time we should return an
error code instead of 0, and HCLGE_STATE_CMD_DISABLE needs to be set to
prevent sending command again.
Fixes: 3ff504908f95 ("net: hns3: fix a dead loop in hclge_cmd_csq_clean")
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>wq
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
index 165c3d5..ac13cb2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
@@ -147,7 +147,12 @@ static int hclge_cmd_csq_clean(struct hclge_hw *hw)
if (!is_valid_csq_clean_head(csq, head)) {
dev_warn(&hdev->pdev->dev, "wrong cmd head (%d, %d-%d)\n", head,
csq->next_to_use, csq->next_to_clean);
- return 0;
+ dev_warn(&hdev->pdev->dev,
+ "Disabling any further commands to IMP firmware\n");
+ set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
+ dev_warn(&hdev->pdev->dev,
+ "IMP firmware watchdog reset soon expected!\n");
+ return -EIO;
}
clean = (head - csq->next_to_clean + csq->desc_num) % csq->desc_num;
@@ -267,10 +272,11 @@ int hclge_cmd_send(struct hclge_hw *hw, struct hclge_desc *desc, int num)
/* Clean the command send queue */
handle = hclge_cmd_csq_clean(hw);
- if (handle != num) {
+ if (handle < 0)
+ retval = handle;
+ else if (handle != num)
dev_warn(&hdev->pdev->dev,
"cleaned %d, need to clean %d\n", handle, num);
- }
spin_unlock_bh(&hw->cmq.csq.lock);
--
2.7.4
^ permalink raw reply related
* AW: AW: AW: PROBLEM: Kernel Oops in UDP stack
From: Marcel Hellwig @ 2018-08-01 11:31 UTC (permalink / raw)
To: 'Eric Dumazet', Paolo Abeni,
'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
Cc: 'netdev@vger.kernel.org',
'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <b727135a-a428-72bc-fa04-dbd64095b21e@gmail.com>
> I suspect the following patch my need to be backported, please Marcel git it a try.
>
The patch is already in 3.4.113 which still has the problem (that version is from October 2016, so not that ancient, but old 😊 )
>Another way to spot the problem would be to add a check in pskb_expand_head()
>
I'll try that. More suggestions are welcome.
Thanks for your engagement!
Mit freundlichen Grüßen / With kind regards
Marcel Hellwig
B. Sc. Informatik
Entwickler
m-u-t GmbH
Am Marienhof 2
22880 Wedel
Germany
Phone: +49 4103 9308 - 474
Fax: +49 4103 9308 - 99
mhellwig@mut-group.com
www.mut-group.com
Geschäftsführer (Managing Director): Fabian Peters
Amtsgericht Pinneberg (Commercial Register No.): HRB 10304 PI
USt-IdNr. (VAT-No.): DE228275390
WEEE-Reg-Nr.: DE 72271808
^ permalink raw reply
* Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation)
From: Eric Dumazet @ 2018-08-01 11:28 UTC (permalink / raw)
To: Dmitry Vyukov, Eric Dumazet
Cc: Andrey Ryabinin, Linus Torvalds, Christoph Lameter,
Theodore Ts'o, Jan Kara, linux-ext4, Greg Kroah-Hartman,
Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David Miller, NetFilter, coreteam, Network Development,
Gerrit Renker, dccp, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Dave Airlie
In-Reply-To: <CACT4Y+bLbDunoz+0qB=atbQXJ9Gu3N6+UXPwNnqMbq5RyZu1mQ@mail.gmail.com>
On 08/01/2018 03:34 AM, Dmitry Vyukov wrote:
> On Wed, Aug 1, 2018 at 12:23 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On 08/01/2018 02:03 AM, Andrey Ryabinin wrote:
>>
>>> I can't think of any advantage in not having the constructor.
>>
>> I can't see any advantage adding another indirect call,
>> in RETPOLINE world.
>
> Can you please elaborate what's the problem here?
> If slab ctor call have RETPOLINE, then using ctors more does not
> introduce any security problems and they are not _that_ slow.
They _are_ slow, when we have dozens of them in a code path.
I object "having to add" yet another indirect call, if this can be avoided [*]
If some people want to use ctor, fine, but do not request this.
[*] This can be tricky, but worth the pain.
^ permalink raw reply
* Re: AW: AW: PROBLEM: Kernel Oops in UDP stack
From: Eric Dumazet @ 2018-08-01 11:25 UTC (permalink / raw)
To: Eric Dumazet, Paolo Abeni, Marcel Hellwig,
'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
Cc: 'netdev@vger.kernel.org',
'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <cd53c2fe-5349-c0e2-c6f5-8bcebdef7024@gmail.com>
On 08/01/2018 03:49 AM, Eric Dumazet wrote:
>
>
> On 08/01/2018 03:44 AM, Paolo Abeni wrote:
>> On Wed, 2018-08-01 at 10:35 +0000, Marcel Hellwig wrote:
>>>>> [<c0228adc>] (udp_recvmsg+0x284/0x33c) from [<c02306e0>] (inet_recvmsg+0x38/0x4c): net/ipv4/udp.c:1234
>>>>
>>>> sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
>>>>
>>>> Unaligned access trap (virtual address c14fe63a), so either sin or ip_hdr(skb) are not on a 32bit alignment
>>>>
>>>> Can you produce the disassembly of the trapping instruction ?
>>>
>>> https://gist.github.com/hellow554/6b11c6c0827d5db80a7e66f71f5636ff#file-net_uipv4_udp-lst-L1892-L1895
>>>
>>> sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
>>> c0228ad8: e5943080 ldr r3, [r4, #128] ; 0x80
>>> c0228adc: e593300c ldr r3, [r3, #12]
>>> c0228ae0: e5823004 str r3, [r2, #4]
>>
>> I *think* pskb_trim_rcsum() in __udp4_lib_rcv() can copy the ipv4
>> header to an unaligned address, for cloned skbs. If I understood
>> correctly the relevant socket is a mcast one, so cloned skbs can land
>> there.
>>
>
> kmalloc() should return aligned pointer.
>
> pskb_expand_head() should allocate aligned skb->head
>
> So pskb_expand_head() should keep whatever offset was provided in the source skb
>
> ( Driver called skb_reserve() or similar function)
>
I suspect the following patch my need to be backported, please Marcel git it a try.
Another way to spot the problem would be to add a check in pskb_expand_head()
commit 5e2afba4ecd7931ea06e6fa116ab28e6943dbd42
Author: Paul Guo <ggang@tilera.com>
Date: Mon Nov 14 19:00:54 2011 +0800
netfilter: possible unaligned packet header in ip_route_me_harder
This patch tries to fix the following issue in netfilter:
In ip_route_me_harder(), we invoke pskb_expand_head() that
rellocates new header with additional head room which can break
the alignment of the original packet header.
In one of my NAT test case, the NIC port for internal hosts is
configured with vlan and the port for external hosts is with
general configuration. If we ping an external "unknown" hosts from an
internal host, an icmp packet will be sent. We find that in
icmp_send()->...->ip_route_me_harder()->pskb_expand_head(), hh_len=18
and current headroom (skb_headroom(skb)) of the packet is 16. After
calling pskb_expand_head() the packet header becomes to be unaligned
and then our system (arch/tile) panics immediately.
Signed-off-by: Paul Guo <ggang@tilera.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index 9899619ab9b8db0f9d8d02c8005c0e6bb01fda94..4f47e064e262c2f24e7cb13eacfcebff0fad86a3 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c
@@ -64,7 +64,8 @@ int ip_route_me_harder(struct sk_buff *skb, unsigned addr_type)
/* Change in oif may mean change in hh_len. */
hh_len = skb_dst(skb)->dev->hard_header_len;
if (skb_headroom(skb) < hh_len &&
- pskb_expand_head(skb, hh_len - skb_headroom(skb), 0, GFP_ATOMIC))
+ pskb_expand_head(skb, HH_DATA_ALIGN(hh_len - skb_headroom(skb)),
+ 0, GFP_ATOMIC))
return -1;
return 0;
^ permalink raw reply related
* Re: [patch net-next 2/2] net: sched: fix notifications for action-held chains
From: Jiri Pirko @ 2018-08-01 9:24 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jakub Kicinski, mlxsw
In-Reply-To: <CAM_iQpWp52KbTdvSNOX3jQn=Ttp9+CiZ3JkQE51jEfexaoPH6g@mail.gmail.com>
Wed, Aug 01, 2018 at 06:27:28AM CEST, xiyou.wangcong@gmail.com wrote:
>On Tue, Jul 31, 2018 at 5:10 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Chains that only have action references serve as placeholders.
>> Until a non-action reference is created, user should not be aware
>> of the chain. Also he should not receive any notifications about it.
>> So send notifications for the new chain only in case the chain gets
>> the first non-action reference. Symmetrically to that, when
>> the last non-action reference is dropped, send the notification about
>> deleted chain.
>>
>> Reported-by: Cong Wang <xiyou.wangcong@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>I think __tcf_chain_{get,put}() can be static.
In fact, tcf_chain_get/put could be now static too. Will send v2.
Thanks!
>
>Other than that,
>
>Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
>
>Thanks.
^ permalink raw reply
* Re: AW: AW: PROBLEM: Kernel Oops in UDP stack
From: Eric Dumazet @ 2018-08-01 10:49 UTC (permalink / raw)
To: Paolo Abeni, Marcel Hellwig, 'Eric Dumazet',
'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
Cc: 'netdev@vger.kernel.org',
'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <225373e97b42e0e5ee6210b329f0bbb94055b651.camel@redhat.com>
On 08/01/2018 03:44 AM, Paolo Abeni wrote:
> On Wed, 2018-08-01 at 10:35 +0000, Marcel Hellwig wrote:
>>>> [<c0228adc>] (udp_recvmsg+0x284/0x33c) from [<c02306e0>] (inet_recvmsg+0x38/0x4c): net/ipv4/udp.c:1234
>>>
>>> sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
>>>
>>> Unaligned access trap (virtual address c14fe63a), so either sin or ip_hdr(skb) are not on a 32bit alignment
>>>
>>> Can you produce the disassembly of the trapping instruction ?
>>
>> https://gist.github.com/hellow554/6b11c6c0827d5db80a7e66f71f5636ff#file-net_uipv4_udp-lst-L1892-L1895
>>
>> sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
>> c0228ad8: e5943080 ldr r3, [r4, #128] ; 0x80
>> c0228adc: e593300c ldr r3, [r3, #12]
>> c0228ae0: e5823004 str r3, [r2, #4]
>
> I *think* pskb_trim_rcsum() in __udp4_lib_rcv() can copy the ipv4
> header to an unaligned address, for cloned skbs. If I understood
> correctly the relevant socket is a mcast one, so cloned skbs can land
> there.
>
kmalloc() should return aligned pointer.
pskb_expand_head() should allocate aligned skb->head
So pskb_expand_head() should keep whatever offset was provided in the source skb
( Driver called skb_reserve() or similar function)
^ permalink raw reply
* Re: AW: AW: PROBLEM: Kernel Oops in UDP stack
From: Paolo Abeni @ 2018-08-01 10:44 UTC (permalink / raw)
To: Marcel Hellwig, 'Eric Dumazet',
'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
Cc: 'netdev@vger.kernel.org',
'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <9f9d731107594b368504cbbaf698bc3d@ZCOM03.mut-group.com>
On Wed, 2018-08-01 at 10:35 +0000, Marcel Hellwig wrote:
> > > [<c0228adc>] (udp_recvmsg+0x284/0x33c) from [<c02306e0>] (inet_recvmsg+0x38/0x4c): net/ipv4/udp.c:1234
> >
> > sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
> >
> > Unaligned access trap (virtual address c14fe63a), so either sin or ip_hdr(skb) are not on a 32bit alignment
> >
> > Can you produce the disassembly of the trapping instruction ?
>
> https://gist.github.com/hellow554/6b11c6c0827d5db80a7e66f71f5636ff#file-net_uipv4_udp-lst-L1892-L1895
>
> sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
> c0228ad8: e5943080 ldr r3, [r4, #128] ; 0x80
> c0228adc: e593300c ldr r3, [r3, #12]
> c0228ae0: e5823004 str r3, [r2, #4]
I *think* pskb_trim_rcsum() in __udp4_lib_rcv() can copy the ipv4
header to an unaligned address, for cloned skbs. If I understood
correctly the relevant socket is a mcast one, so cloned skbs can land
there.
Paolo
^ permalink raw reply
* AW: AW: PROBLEM: Kernel Oops in UDP stack
From: Marcel Hellwig @ 2018-08-01 10:35 UTC (permalink / raw)
To: 'Eric Dumazet', 'davem@davemloft.net',
'kuznet@ms2.inr.ac.ru', 'yoshfuji@linux-ipv6.org',
'andrew@lunn.ch'
Cc: 'netdev@vger.kernel.org',
'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <96753ca4-88fe-aa26-b439-6e22eaf8b524@gmail.com>
>> [<c0228adc>] (udp_recvmsg+0x284/0x33c) from [<c02306e0>] (inet_recvmsg+0x38/0x4c): net/ipv4/udp.c:1234
>
> sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
>
>Unaligned access trap (virtual address c14fe63a), so either sin or ip_hdr(skb) are not on a 32bit alignment
>
>Can you produce the disassembly of the trapping instruction ?
https://gist.github.com/hellow554/6b11c6c0827d5db80a7e66f71f5636ff#file-net_uipv4_udp-lst-L1892-L1895
sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
c0228ad8: e5943080 ldr r3, [r4, #128] ; 0x80
c0228adc: e593300c ldr r3, [r3, #12]
c0228ae0: e5823004 str r3, [r2, #4]
Mit freundlichen Grüßen / With kind regards
Marcel Hellwig
B. Sc. Informatik
Entwickler
m-u-t GmbH
Am Marienhof 2
22880 Wedel
Germany
Phone: +49 4103 9308 - 474
Fax: +49 4103 9308 - 99
mhellwig@mut-group.com
www.mut-group.com
Geschäftsführer (Managing Director): Fabian Peters
Amtsgericht Pinneberg (Commercial Register No.): HRB 10304 PI
USt-IdNr. (VAT-No.): DE228275390
WEEE-Reg-Nr.: DE 72271808
^ permalink raw reply
* Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation)
From: Dmitry Vyukov @ 2018-08-01 10:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andrey Ryabinin, Linus Torvalds, Christoph Lameter,
Theodore Ts'o, Jan Kara, linux-ext4, Greg Kroah-Hartman,
Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David Miller, NetFilter, coreteam, Network Development,
Gerrit Renker, dccp, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Dave Airlie
In-Reply-To: <c03fd1ca-0169-4492-7d6f-2df7a91bff5e@gmail.com>
On Wed, Aug 1, 2018 at 12:23 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On 08/01/2018 02:03 AM, Andrey Ryabinin wrote:
>
>> I can't think of any advantage in not having the constructor.
>
> I can't see any advantage adding another indirect call,
> in RETPOLINE world.
Can you please elaborate what's the problem here?
If slab ctor call have RETPOLINE, then using ctors more does not
introduce any security problems and they are not _that_ slow.
If slab ctors are not protected, then we have problem that needs to be
fixed already.
What am I missing?
^ permalink raw reply
* Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation)
From: Dmitry Vyukov @ 2018-08-01 8:46 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christoph Lameter, Andrey Ryabinin, Theodore Ts'o, Jan Kara,
linux-ext4, Greg Kroah-Hartman, Pablo Neira Ayuso,
Jozsef Kadlecsik, Florian Westphal, David Miller, NetFilter,
coreteam, Network Development, Gerrit Renker, dccp, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Dave Airlie, intel-gfx
In-Reply-To: <CA+55aFzYLgyNp1jsqsvUOjwZdO_1Piqj=iB=rzDShjScdNtkbg@mail.gmail.com>
On Tue, Jul 31, 2018 at 8:51 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Jul 31, 2018 at 10:49 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> So the re-use might initialize the fields lazily, not necessarily using a ctor.
>
> In particular, the pattern that nf_conntrack uses looks like it is safe.
>
> If you have a well-defined refcount, and use "atomic_inc_not_zero()"
> to guard the speculative RCU access section, and use
> "atomic_dec_and_test()" in the freeing section, then you should be
> safe wrt new allocations.
>
> If you have a completely new allocation that has "random stale
> content", you know that it cannot be on the RCU list, so there is no
> speculative access that can ever see that random content.
>
> So the only case you need to worry about is a re-use allocation, and
> you know that the refcount will start out as zero even if you don't
> have a constructor.
>
> So you can think of the refcount itself as always having a zero
> constructor, *BUT* you need to be careful with ordering.
>
> In particular, whoever does the allocation needs to then set the
> refcount to a non-zero value *after* it has initialized all the other
> fields. And in particular, it needs to make sure that it uses the
> proper memory ordering to do so.
>
> And in this case, we have
>
> static struct nf_conn *
> __nf_conntrack_alloc(struct net *net,
> {
> ...
> atomic_set(&ct->ct_general.use, 0);
>
> which is a no-op for the re-use case (whether racing or not, since any
> "inc_not_zero" users won't touch it), but initializes it to zero for
> the "completely new object" case.
>
> And then, the thing that actually exposes it to the speculative walkers does:
>
> int
> nf_conntrack_hash_check_insert(struct nf_conn *ct)
> {
> ...
> smp_wmb();
> /* The caller holds a reference to this object */
> atomic_set(&ct->ct_general.use, 2);
>
> which means that it stays as zero until everything is actually set up,
> and then the optimistic walker can use the other fields (including
> spinlocks etc) to verify that it's actually the right thing. The
> smp_wmb() means that the previous initialization really will be
> visible before the object is visible.
>
> Side note: on some architectures it might help to make that "smp_wmb
> -> atomic_set()" sequence be am "smp_store_release()" instead. Doesn't
> matter on x86, but might matter on arm64.
>
> NOTE! One thing to be very worried about is that re-initializing
> whatever RCU lists means that now the RCU walker may be walking on the
> wrong list so the walker may do the right thing for this particular
> entry, but it may miss walking *other* entries. So then you can get
> spurious lookup failures, because the RCU walker never walked all the
> way to the end of the right list. That ends up being a much more
> subtle bug.
>
> But the nf_conntrack case seems to get that right too, see the restart
> in ____nf_conntrack_find().
>
> So I don't see anything wrong in nf_conntrack.
>
> But yes, using SLAB_TYPESAFE_BY_RCU is very very subtle. But most of
> the subtleties have nothing to do with having a constructor, they are
> about those "make sure memory ordering wrt refcount is right" and
> "restart speculative RCU walk" issues that actually happen regardless
> of having a constructor or not.
Thank you, this is very enlightening.
So the type-stable invariant is established by initialization of the
object after the first kmem_cache_alloc, and then we rely on the fact
that repeated initialization does not break the invariant, which works
because the state is very simple (including debug builds, i.e. no
ODEBUG nor magic values).
There is a bunch of other SLAB_TYPESAFE_BY_RCU uses without ctor:
https://elixir.bootlin.com/linux/latest/source/fs/jbd2/journal.c#L2395
https://elixir.bootlin.com/linux/latest/source/fs/kernfs/mount.c#L415
https://elixir.bootlin.com/linux/latest/source/net/netfilter/nf_conntrack_core.c#L2065
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/i915_gem.c#L5501
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/selftests/mock_gem_device.c#L212
https://elixir.bootlin.com/linux/latest/source/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c#L1131
Also these in proto structs:
https://elixir.bootlin.com/linux/latest/source/net/dccp/ipv4.c#L959
https://elixir.bootlin.com/linux/latest/source/net/dccp/ipv6.c#L1048
https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp_ipv4.c#L2461
https://elixir.bootlin.com/linux/latest/source/net/ipv6/tcp_ipv6.c#L1980
https://elixir.bootlin.com/linux/latest/source/net/llc/af_llc.c#L145
https://elixir.bootlin.com/linux/latest/source/net/smc/af_smc.c#L105
They later created in net/core/sock.c without ctor.
I guess it would be useful to have such extensive comment for each
SLAB_TYPESAFE_BY_RCU use explaining why it is needed and how all the
tricky aspects are handled.
For example, the one in jbd2 is interesting because it memsets the
whole object before freeing it into SLAB_TYPESAFE_BY_RCU slab:
memset(jh, JBD2_POISON_FREE, sizeof(*jh));
kmem_cache_free(jbd2_journal_head_cache, jh);
I guess there are also tricky ways how it can all work in the end
(type-stable state is only a byte, or we check for all possible
combinations of being overwritten with JBD2_POISON_FREE). But at first
sight it does look fishy.
^ permalink raw reply
* Re: SLAB_TYPESAFE_BY_RCU without constructors (was Re: [PATCH v4 13/17] khwasan: add hooks implementation)
From: Eric Dumazet @ 2018-08-01 10:23 UTC (permalink / raw)
To: Andrey Ryabinin, Linus Torvalds, Christoph Lameter
Cc: Theodore Ts'o, Jan Kara, linux-ext4, Greg Kroah-Hartman,
Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David Miller, NetFilter, coreteam, Network Development, gerrit,
dccp, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Dave Airlie,
intel-gfx, DRI, Eric Dumazet, Alexey Kuznetsov <
In-Reply-To: <30ee6c72-dc90-275a-8e23-54221f393cb0@virtuozzo.com>
On 08/01/2018 02:03 AM, Andrey Ryabinin wrote:
> I can't think of any advantage in not having the constructor.
>
I can't see any advantage adding another indirect call,
in RETPOLINE world.
^ permalink raw reply
* Re: AW: PROBLEM: Kernel Oops in UDP stack
From: Eric Dumazet @ 2018-08-01 10:20 UTC (permalink / raw)
To: Marcel Hellwig, 'Eric Dumazet',
'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
Cc: 'netdev@vger.kernel.org',
'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <b8ceab2531f345d295686903b003787b@ZCOM03.mut-group.com>
On 07/31/2018 10:55 PM, Marcel Hellwig wrote:
>
> [<c0228adc>] (udp_recvmsg+0x284/0x33c) from [<c02306e0>] (inet_recvmsg+0x38/0x4c): net/ipv4/udp.c:1234
sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
Unaligned access trap (virtual address c14fe63a), so either sin or ip_hdr(skb) are not on a 32bit alignment
Can you produce the disassembly of the trapping instruction ?
(Is is a read at address c14fe63a, or a write ...)
Thanks.
^ permalink raw reply
* Re: [net-next V2 12/12] net/mlx5e: Use PARTIAL_GSO for UDP segmentation
From: Boris Pismenny @ 2018-08-01 8:32 UTC (permalink / raw)
To: Saeed Mahameed, Alexander Duyck; +Cc: Saeed Mahameed, David S. Miller, Netdev
In-Reply-To: <CALzJLG_Mh2xWwZzWWO5YnKmQyan=RVrLLtrPJHUFAvd-xKoVpA@mail.gmail.com>
On 7/24/2018 8:35 PM, Saeed Mahameed wrote:
> On Tue, Jul 24, 2018 at 7:53 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
>> On Mon, Jul 23, 2018 at 3:11 PM, Saeed Mahameed <saeedm@mellanox.com> wrote:
>>> From: Boris Pismenny <borisp@mellanox.com>
>>>
>>> This patch removes the splitting of UDP_GSO_L4 packets in the driver,
>>> and exposes UDP_GSO_L4 as a PARTIAL_GSO feature. Thus, the network stack
>>> is not responsible for splitting the packet into two.
>>>
>>> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
>>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>>> ---
>>> .../net/ethernet/mellanox/mlx5/core/Makefile | 4 +-
>>> .../mellanox/mlx5/core/en_accel/en_accel.h | 27 +++--
>>> .../mellanox/mlx5/core/en_accel/rxtx.c | 109 ------------------
>>> .../mellanox/mlx5/core/en_accel/rxtx.h | 14 ---
>>> .../net/ethernet/mellanox/mlx5/core/en_main.c | 9 +-
>>> 5 files changed, 23 insertions(+), 140 deletions(-)
>>> delete mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
>>> delete mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.h
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>>> index 55d5a5c2e9d8..fa7fcca5dc78 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>>> @@ -14,8 +14,8 @@ mlx5_core-$(CONFIG_MLX5_FPGA) += fpga/cmd.o fpga/core.o fpga/conn.o fpga/sdk.o \
>>> fpga/ipsec.o fpga/tls.o
>>>
>>> mlx5_core-$(CONFIG_MLX5_CORE_EN) += en_main.o en_common.o en_fs.o en_ethtool.o \
>>> - en_tx.o en_rx.o en_dim.o en_txrx.o en_accel/rxtx.o en_stats.o \
>>> - vxlan.o en_arfs.o en_fs_ethtool.o en_selftest.o en/port.o
>>> + en_tx.o en_rx.o en_dim.o en_txrx.o en_stats.o vxlan.o \
>>> + en_arfs.o en_fs_ethtool.o en_selftest.o en/port.o
>>>
>>> mlx5_core-$(CONFIG_MLX5_MPFS) += lib/mpfs.o
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
>>> index 39a5d13ba459..1dd225380a66 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
>>> @@ -38,14 +38,22 @@
>>> #include <linux/netdevice.h>
>>> #include "en_accel/ipsec_rxtx.h"
>>> #include "en_accel/tls_rxtx.h"
>>> -#include "en_accel/rxtx.h"
>>> #include "en.h"
>>>
>>> -static inline struct sk_buff *mlx5e_accel_handle_tx(struct sk_buff *skb,
>>> - struct mlx5e_txqsq *sq,
>>> - struct net_device *dev,
>>> - struct mlx5e_tx_wqe **wqe,
>>> - u16 *pi)
>>> +static inline void
>>> +mlx5e_udp_gso_handle_tx_skb(struct sk_buff *skb)
>>> +{
>>> + int payload_len = skb_shinfo(skb)->gso_size + sizeof(struct udphdr);
>>> +
>>> + udp_hdr(skb)->len = htons(payload_len);
>>> +}
>>> +
>>
>> So it looks like you decided to just update the length here. Do you
>> still have plans to update GSO_PARTIAL to set the length this way or
>> have you decided to just leave it as it is?
>>
>
> I don't know what are Boris's plans regarding this, I will let him answer that.
> But what is your take on this ?
>
Sorry for the late response. I'm currently busy with other things. I'd
rather let it be like this for now, and fix it later. If you'd like to
change it, I'll be happy to test.
Thanks
^ permalink raw reply
* [PATCH] net: hns: remove redundant variables 'max_frm' and 'tmp_mac_key'
From: YueHaibing @ 2018-08-01 10:16 UTC (permalink / raw)
To: davem, yisen.zhuang, salil.mehta, lipeng321
Cc: linux-kernel, netdev, shenjian15, joe, keescook, wangxi11,
YueHaibing
Variables 'max_frm' and 'tmp_mac_key' are being assigned,
but are never used,hence they are redundant and can be removed.
fix fllowing warning:
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c:461:6: warning: variable 'max_frm' set but not used [-Wunused-but-set-variable]
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:1685:31: warning: variable 'tmp_mac_key' set but not used [-Wunused-but-set-variable]
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:1855:41: warning: variable 'tmp_mac_key' set but not used [-Wunused-but-set-variable]
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 5 -----
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 10 +---------
2 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index 6e5107d..3545a5d 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -458,11 +458,6 @@ int hns_mac_set_mtu(struct hns_mac_cb *mac_cb, u32 new_mtu, u32 buf_size)
{
struct mac_driver *drv = hns_mac_get_drv(mac_cb);
u32 new_frm = new_mtu + ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN;
- u32 max_frm = AE_IS_VER1(mac_cb->dsaf_dev->dsaf_ver) ?
- MAC_MAX_MTU : MAC_MAX_MTU_V2;
-
- if (mac_cb->mac_type == HNAE_PORT_DEBUG)
- max_frm = MAC_MAX_MTU_DBG;
if (new_frm > HNS_RCB_RING_MAX_BD_PER_PKT * buf_size)
return -EINVAL;
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index 619e6ce..ca50c25 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -1683,7 +1683,6 @@ int hns_dsaf_add_mac_mc_port(struct dsaf_device *dsaf_dev,
struct dsaf_tbl_tcam_mcast_cfg mac_data;
struct dsaf_drv_priv *priv = hns_dsaf_dev_priv(dsaf_dev);
struct dsaf_drv_soft_mac_tbl *soft_mac_entry = priv->soft_mac_tbl;
- struct dsaf_drv_tbl_tcam_key tmp_mac_key;
struct dsaf_tbl_tcam_data tcam_data;
u8 mc_addr[ETH_ALEN];
int mskid;
@@ -1740,10 +1739,6 @@ int hns_dsaf_add_mac_mc_port(struct dsaf_device *dsaf_dev,
/* if exist, add in */
hns_dsaf_tcam_mc_get(dsaf_dev, entry_index, &tcam_data,
&mac_data);
-
- tmp_mac_key.high.val =
- le32_to_cpu(tcam_data.tbl_tcam_data_high);
- tmp_mac_key.low.val = le32_to_cpu(tcam_data.tbl_tcam_data_low);
}
/* config hardware entry */
@@ -1853,7 +1848,7 @@ int hns_dsaf_del_mac_mc_port(struct dsaf_device *dsaf_dev,
struct dsaf_tbl_tcam_data tcam_data;
int mskid;
const u8 empty_msk[sizeof(mac_data.tbl_mcast_port_msk)] = {0};
- struct dsaf_drv_tbl_tcam_key mask_key, tmp_mac_key;
+ struct dsaf_drv_tbl_tcam_key mask_key;
struct dsaf_tbl_tcam_data *pmask_key = NULL;
u8 mc_addr[ETH_ALEN];
@@ -1916,9 +1911,6 @@ int hns_dsaf_del_mac_mc_port(struct dsaf_device *dsaf_dev,
/* read entry */
hns_dsaf_tcam_mc_get(dsaf_dev, entry_index, &tcam_data, &mac_data);
- tmp_mac_key.high.val = le32_to_cpu(tcam_data.tbl_tcam_data_high);
- tmp_mac_key.low.val = le32_to_cpu(tcam_data.tbl_tcam_data_low);
-
/*del the port*/
if (mac_entry->port_num < DSAF_SERVICE_NW_NUM) {
mskid = mac_entry->port_num;
--
2.7.0
^ permalink raw reply related
* [v2, 3/3] ptp_qoriq: support automatic configuration for ptp timer
From: Yangbo Lu @ 2018-08-01 10:05 UTC (permalink / raw)
To: netdev, madalin.bucur, Richard Cochran, Rob Herring, Shawn Guo,
David S . Miller
Cc: devicetree, linuxppc-dev, linux-arm-kernel, linux-kernel,
Yangbo Lu
In-Reply-To: <20180801100554.36634-1-yangbo.lu@nxp.com>
This patch is to support automatic configuration for ptp timer.
If required ptp dts properties are not provided, driver could
try to calculate a set of default configurations to initialize
the ptp timer. This makes the driver work for many boards which
don't have the required ptp dts properties in current kernel.
Also the users could set dts properties by themselves according
to their requirement.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
- Dropped module_param.
---
drivers/ptp/ptp_qoriq.c | 111 +++++++++++++++++++++++++++++++++++++++-
include/linux/fsl/ptp_qoriq.h | 6 ++-
2 files changed, 113 insertions(+), 4 deletions(-)
diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c
index a14c317..095c185 100644
--- a/drivers/ptp/ptp_qoriq.c
+++ b/drivers/ptp/ptp_qoriq.c
@@ -29,6 +29,7 @@
#include <linux/of_platform.h>
#include <linux/timex.h>
#include <linux/slab.h>
+#include <linux/clk.h>
#include <linux/fsl/ptp_qoriq.h>
@@ -317,6 +318,105 @@ static int ptp_qoriq_enable(struct ptp_clock_info *ptp,
.enable = ptp_qoriq_enable,
};
+/**
+ * qoriq_ptp_nominal_freq - calculate nominal frequency according to
+ * reference clock frequency
+ *
+ * @clk_src: reference clock frequency
+ *
+ * The nominal frequency is the desired clock frequency.
+ * It should be less than the reference clock frequency.
+ * It should be a factor of 1000MHz.
+ *
+ * Return the nominal frequency
+ */
+static u32 qoriq_ptp_nominal_freq(u32 clk_src)
+{
+ u32 remainder = 0;
+
+ clk_src /= 1000000;
+ remainder = clk_src % 100;
+ if (remainder) {
+ clk_src -= remainder;
+ clk_src += 100;
+ }
+
+ do {
+ clk_src -= 100;
+
+ } while (1000 % clk_src);
+
+ return clk_src * 1000000;
+}
+
+/**
+ * qoriq_ptp_auto_config - calculate a set of default configurations
+ *
+ * @qoriq_ptp: pointer to qoriq_ptp
+ * @node: pointer to device_node
+ *
+ * If below dts properties are not provided, this function will be
+ * called to calculate a set of default configurations for them.
+ * "fsl,tclk-period"
+ * "fsl,tmr-prsc"
+ * "fsl,tmr-add"
+ * "fsl,tmr-fiper1"
+ * "fsl,tmr-fiper2"
+ * "fsl,max-adj"
+ *
+ * Return 0 if success
+ */
+static int qoriq_ptp_auto_config(struct qoriq_ptp *qoriq_ptp,
+ struct device_node *node)
+{
+ struct clk *clk;
+ u64 freq_comp;
+ u64 max_adj;
+ u32 nominal_freq;
+ u32 clk_src = 0;
+
+ qoriq_ptp->cksel = DEFAULT_CKSEL;
+
+ clk = of_clk_get(node, 0);
+ if (!IS_ERR(clk)) {
+ clk_src = clk_get_rate(clk);
+ clk_put(clk);
+ }
+
+ if (clk_src <= 100000000UL) {
+ pr_err("error reference clock value, or lower than 100MHz\n");
+ return -EINVAL;
+ }
+
+ nominal_freq = qoriq_ptp_nominal_freq(clk_src);
+ if (!nominal_freq)
+ return -EINVAL;
+
+ qoriq_ptp->tclk_period = 1000000000UL / nominal_freq;
+ qoriq_ptp->tmr_prsc = DEFAULT_TMR_PRSC;
+
+ /* Calculate initial frequency compensation value for TMR_ADD register.
+ * freq_comp = ceil(2^32 / freq_ratio)
+ * freq_ratio = reference_clock_freq / nominal_freq
+ */
+ freq_comp = ((u64)1 << 32) * nominal_freq;
+ if (do_div(freq_comp, clk_src))
+ freq_comp++;
+
+ qoriq_ptp->tmr_add = freq_comp;
+ qoriq_ptp->tmr_fiper1 = DEFAULT_FIPER1_PERIOD - qoriq_ptp->tclk_period;
+ qoriq_ptp->tmr_fiper2 = DEFAULT_FIPER2_PERIOD - qoriq_ptp->tclk_period;
+
+ /* max_adj = 1000000000 * (freq_ratio - 1.0) - 1
+ * freq_ratio = reference_clock_freq / nominal_freq
+ */
+ max_adj = 1000000000ULL * (clk_src - nominal_freq);
+ max_adj = max_adj / nominal_freq - 1;
+ qoriq_ptp->caps.max_adj = max_adj;
+
+ return 0;
+}
+
static int qoriq_ptp_probe(struct platform_device *dev)
{
struct device_node *node = dev->dev.of_node;
@@ -332,7 +432,7 @@ static int qoriq_ptp_probe(struct platform_device *dev)
if (!qoriq_ptp)
goto no_memory;
- err = -ENODEV;
+ err = -EINVAL;
qoriq_ptp->caps = ptp_qoriq_caps;
@@ -351,10 +451,14 @@ static int qoriq_ptp_probe(struct platform_device *dev)
"fsl,tmr-fiper2", &qoriq_ptp->tmr_fiper2) ||
of_property_read_u32(node,
"fsl,max-adj", &qoriq_ptp->caps.max_adj)) {
- pr_err("device tree node missing required elements\n");
- goto no_node;
+ pr_warn("device tree node missing required elements, try automatic configuration\n");
+
+ if (qoriq_ptp_auto_config(qoriq_ptp, node))
+ goto no_config;
}
+ err = -ENODEV;
+
qoriq_ptp->irq = platform_get_irq(dev, 0);
if (qoriq_ptp->irq < 0) {
@@ -436,6 +540,7 @@ static int qoriq_ptp_probe(struct platform_device *dev)
release_resource(qoriq_ptp->rsrc);
no_resource:
free_irq(qoriq_ptp->irq, qoriq_ptp);
+no_config:
no_node:
kfree(qoriq_ptp);
no_memory:
diff --git a/include/linux/fsl/ptp_qoriq.h b/include/linux/fsl/ptp_qoriq.h
index dc3dac4..c1f003a 100644
--- a/include/linux/fsl/ptp_qoriq.h
+++ b/include/linux/fsl/ptp_qoriq.h
@@ -127,9 +127,13 @@ struct qoriq_ptp_registers {
#define DRIVER "ptp_qoriq"
-#define DEFAULT_CKSEL 1
#define N_EXT_TS 2
+#define DEFAULT_CKSEL 1
+#define DEFAULT_TMR_PRSC 2
+#define DEFAULT_FIPER1_PERIOD 1000000000
+#define DEFAULT_FIPER2_PERIOD 100000
+
struct qoriq_ptp {
void __iomem *base;
struct qoriq_ptp_registers regs;
--
1.7.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox