* [iproute2 1/1] tipc: TIPC_NLA_LINK_NAME value pass on nesting entry TIPC_NLA_LINK
From: Hoang Le @ 2018-06-08 2:19 UTC (permalink / raw)
To: netdev, tipc-discussion, jon.maloy, maloy, ying.xue
In the commit 94f6a80 on next-net, TIPC_NLA_LINK_NAME attribute should be
retrieved and validated via TIPC_NLA_LINK nesting entry in
tipc_nl_node_get_link().
According to that commit, TIPC_NLA_LINK_NAME value passing via
tipc link get command must follow above hierachy.
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
tipc/link.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tipc/link.c b/tipc/link.c
index 02f14aadefa6..a2d7c0016bc1 100644
--- a/tipc/link.c
+++ b/tipc/link.c
@@ -97,6 +97,7 @@ static int cmd_link_get_prop(struct nlmsghdr *nlh, const struct cmd *cmd,
{
int prop;
char buf[MNL_SOCKET_BUFFER_SIZE];
+ struct nlattr *attrs;
struct opt *opt;
struct opt opts[] = {
{ "link", OPT_KEYVAL, NULL },
@@ -131,7 +132,9 @@ static int cmd_link_get_prop(struct nlmsghdr *nlh, const struct cmd *cmd,
fprintf(stderr, "error, missing link\n");
return -EINVAL;
}
+ attrs = mnl_attr_nest_start(nlh, TIPC_NLA_LINK);
mnl_attr_put_strz(nlh, TIPC_NLA_LINK_NAME, opt->val);
+ mnl_attr_nest_end(nlh, attrs);
return msg_doit(nlh, link_get_cb, &prop);
}
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* Re: [PATCH net-next] net: ipv6: Generate random IID for addresses on RAWIP devices
From: Lorenzo Colitti @ 2018-06-08 2:52 UTC (permalink / raw)
To: YOSHIFUJI Hideaki
Cc: Subash Abhinov Kasiviswanathan, David Miller, netdev,
YOSHIFUJI Hideaki
In-Reply-To: <CAPA1RqBmffQObdTWMxNSNMRTgM9gPjD4KGqk2+MTwdbR34Wkhw@mail.gmail.com>
On Mon, Jun 4, 2018 at 8:51 AM 吉藤英明 <hideaki.yoshifuji@miraclelinux.com> wrote:
>
> > + if (ipv6_get_lladdr(dev, &lladdr, IFA_F_TENTATIVE))
> > + get_random_bytes(eui, 8);
>
> Please be aware of I/G bit and G/L bit.
Actually, I think this is fine. RFC 7136 clarified this, and says:
======
Thus, we can conclude that the value of the "u" bit in IIDs has no
particular meaning. In the case of an IID created from a MAC address
according to RFC 4291, its value is determined by the MAC address,
but that is all.
[...]
Specifications of other forms of 64-bit IIDs MUST specify how all 64
bits are set, but a generic semantic meaning for the "u" and "g" bits
MUST NOT be defined. However, the method of generating IIDs for
specific link types MAY define some local significance for certain
bits.
In all cases, the bits in an IID have no generic semantics; in other
words, they have opaque values. In fact, the whole IID value MUST be
viewed as an opaque bit string by third parties, except possibly in
the local context.
======
That said - we already have a way to form all-random IIDs:
IN6_ADDR_GEN_MODE_RANDOM. Can't you just ensure that links of type
ARPHRD_RAWIP always use IN6_ADDR_GEN_MODE_RANDOM? That might lead to
less special-casing.
^ permalink raw reply
* [PATCH v2 net] net: fddi: fix a possible null-ptr-deref
From: YueHaibing @ 2018-06-08 2:58 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, YueHaibing
bp->SharedMemAddr is set to NULL while bp->SharedMemSize lesser-or-equal 0,
then memset will trigger null-ptr-deref.
fix it by replacing pci_alloc_consistent with dma_zalloc_coherent.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
v1->v2: move from pci_dma* to dma_* as Christoph suggested
---
drivers/net/fddi/skfp/skfddi.c | 55 +++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/drivers/net/fddi/skfp/skfddi.c b/drivers/net/fddi/skfp/skfddi.c
index 2414f1d..72433f3e 100644
--- a/drivers/net/fddi/skfp/skfddi.c
+++ b/drivers/net/fddi/skfp/skfddi.c
@@ -297,11 +297,11 @@ static int skfp_init_one(struct pci_dev *pdev,
return 0;
err_out5:
if (smc->os.SharedMemAddr)
- pci_free_consistent(pdev, smc->os.SharedMemSize,
- smc->os.SharedMemAddr,
- smc->os.SharedMemDMA);
- pci_free_consistent(pdev, MAX_FRAME_SIZE,
- smc->os.LocalRxBuffer, smc->os.LocalRxBufferDMA);
+ dma_free_coherent(&pdev->dev, smc->os.SharedMemSize,
+ smc->os.SharedMemAddr,
+ smc->os.SharedMemDMA);
+ dma_free_coherent(&pdev->dev, MAX_FRAME_SIZE,
+ smc->os.LocalRxBuffer, smc->os.LocalRxBufferDMA);
err_out4:
free_netdev(dev);
err_out3:
@@ -328,17 +328,17 @@ static void skfp_remove_one(struct pci_dev *pdev)
unregister_netdev(p);
if (lp->os.SharedMemAddr) {
- pci_free_consistent(&lp->os.pdev,
- lp->os.SharedMemSize,
- lp->os.SharedMemAddr,
- lp->os.SharedMemDMA);
+ dma_free_coherent(&pdev->dev,
+ lp->os.SharedMemSize,
+ lp->os.SharedMemAddr,
+ lp->os.SharedMemDMA);
lp->os.SharedMemAddr = NULL;
}
if (lp->os.LocalRxBuffer) {
- pci_free_consistent(&lp->os.pdev,
- MAX_FRAME_SIZE,
- lp->os.LocalRxBuffer,
- lp->os.LocalRxBufferDMA);
+ dma_free_coherent(&pdev->dev,
+ MAX_FRAME_SIZE,
+ lp->os.LocalRxBuffer,
+ lp->os.LocalRxBufferDMA);
lp->os.LocalRxBuffer = NULL;
}
#ifdef MEM_MAPPED_IO
@@ -394,7 +394,9 @@ static int skfp_driver_init(struct net_device *dev)
spin_lock_init(&bp->DriverLock);
// Allocate invalid frame
- bp->LocalRxBuffer = pci_alloc_consistent(&bp->pdev, MAX_FRAME_SIZE, &bp->LocalRxBufferDMA);
+ bp->LocalRxBuffer = dma_alloc_coherent(&bp->pdev.dev, MAX_FRAME_SIZE,
+ &bp->LocalRxBufferDMA,
+ GFP_ATOMIC);
if (!bp->LocalRxBuffer) {
printk("could not allocate mem for ");
printk("LocalRxBuffer: %d byte\n", MAX_FRAME_SIZE);
@@ -407,23 +409,22 @@ static int skfp_driver_init(struct net_device *dev)
if (bp->SharedMemSize > 0) {
bp->SharedMemSize += 16; // for descriptor alignment
- bp->SharedMemAddr = pci_alloc_consistent(&bp->pdev,
- bp->SharedMemSize,
- &bp->SharedMemDMA);
+ bp->SharedMemAddr = dma_zalloc_coherent(&bp->pdev.dev,
+ bp->SharedMemSize,
+ &bp->SharedMemDMA,
+ GFP_ATOMIC);
if (!bp->SharedMemAddr) {
printk("could not allocate mem for ");
printk("hardware module: %ld byte\n",
bp->SharedMemSize);
goto fail;
}
- bp->SharedMemHeap = 0; // Nothing used yet.
} else {
bp->SharedMemAddr = NULL;
- bp->SharedMemHeap = 0;
- } // SharedMemSize > 0
+ }
- memset(bp->SharedMemAddr, 0, bp->SharedMemSize);
+ bp->SharedMemHeap = 0;
card_stop(smc); // Reset adapter.
@@ -442,15 +443,15 @@ static int skfp_driver_init(struct net_device *dev)
fail:
if (bp->SharedMemAddr) {
- pci_free_consistent(&bp->pdev,
- bp->SharedMemSize,
- bp->SharedMemAddr,
- bp->SharedMemDMA);
+ dma_free_coherent(&bp->pdev.dev,
+ bp->SharedMemSize,
+ bp->SharedMemAddr,
+ bp->SharedMemDMA);
bp->SharedMemAddr = NULL;
}
if (bp->LocalRxBuffer) {
- pci_free_consistent(&bp->pdev, MAX_FRAME_SIZE,
- bp->LocalRxBuffer, bp->LocalRxBufferDMA);
+ dma_free_coherent(&bp->pdev.dev, MAX_FRAME_SIZE,
+ bp->LocalRxBuffer, bp->LocalRxBufferDMA);
bp->LocalRxBuffer = NULL;
}
return err;
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net] net/sched: act_simple: fix parsing of TCA_DEFDATA
From: Davide Caratti @ 2018-06-08 3:00 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem
In-Reply-To: <20180608020754.GT8958@leo.usersys.redhat.com>
On Fri, 2018-06-08 at 10:07 +0800, Hangbin Liu wrote:
> On Thu, Jun 07, 2018 at 03:46:43PM +0200, Davide Caratti wrote:
> > use nla_strlcpy() to avoid copying data beyond the length of TCA_DEFDATA
>
> s/TCA_DEFDATA/TCA_DEF_DATA/, incase someone search the commit history but
> could not find it.
>
> Thanks
> Hangbin
sure, thanks, and after another look I think also the 'Fixes:' tag is
wrong. More probably it was introduced with
fa1b1cff3d06 "net_cls_act: Make act_simple use of netlink policy."
I will send a v2 in minutes.
regards,
--
davide
^ permalink raw reply
* [PATCH net v2] net/sched: act_simple: fix parsing of TCA_DEF_DATA
From: Davide Caratti @ 2018-06-08 3:02 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko; +Cc: David S. Miller, netdev
use nla_strlcpy() to avoid copying data beyond the length of TCA_DEF_DATA
netlink attribute, in case it is less than SIMP_MAX_DATA and it does not
end with '\0' character.
v2: fix errors in the commit message, thanks Hangbin Liu
Fixes: fa1b1cff3d06 ("net_cls_act: Make act_simple use of netlink policy.")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_simple.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 9618b4a83cee..98c4afe7c15b 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -53,22 +53,22 @@ static void tcf_simp_release(struct tc_action *a)
kfree(d->tcfd_defdata);
}
-static int alloc_defdata(struct tcf_defact *d, char *defdata)
+static int alloc_defdata(struct tcf_defact *d, const struct nlattr *defdata)
{
d->tcfd_defdata = kzalloc(SIMP_MAX_DATA, GFP_KERNEL);
if (unlikely(!d->tcfd_defdata))
return -ENOMEM;
- strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
+ nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
return 0;
}
-static void reset_policy(struct tcf_defact *d, char *defdata,
+static void reset_policy(struct tcf_defact *d, const struct nlattr *defdata,
struct tc_defact *p)
{
spin_lock_bh(&d->tcf_lock);
d->tcf_action = p->action;
memset(d->tcfd_defdata, 0, SIMP_MAX_DATA);
- strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
+ nla_strlcpy(d->tcfd_defdata, defdata, SIMP_MAX_DATA);
spin_unlock_bh(&d->tcf_lock);
}
@@ -87,7 +87,6 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
struct tcf_defact *d;
bool exists = false;
int ret = 0, err;
- char *defdata;
if (nla == NULL)
return -EINVAL;
@@ -110,8 +109,6 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
return -EINVAL;
}
- defdata = nla_data(tb[TCA_DEF_DATA]);
-
if (!exists) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_simp_ops, bind, false);
@@ -119,7 +116,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
return ret;
d = to_defact(*a);
- ret = alloc_defdata(d, defdata);
+ ret = alloc_defdata(d, tb[TCA_DEF_DATA]);
if (ret < 0) {
tcf_idr_release(*a, bind);
return ret;
@@ -133,7 +130,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
if (!ovr)
return -EEXIST;
- reset_policy(d, defdata, parm);
+ reset_policy(d, tb[TCA_DEF_DATA], parm);
}
if (ret == ACT_P_CREATED)
--
2.17.0
^ permalink raw reply related
* Re: linux-next: manual merge of the scsi tree with the net-next tree
From: Stephen Rothwell @ 2018-06-08 3:11 UTC (permalink / raw)
To: James Bottomley, Martin K. Petersen, linux-scsi
Cc: Mark Brown, Chad Dupuis, David S. Miller, netdev,
Linux-Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20180525013810.GG4828@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]
Hi all,
On Fri, 25 May 2018 02:38:10 +0100 Mark Brown <broonie@kernel.org> wrote:
>
> Today's linux-next merge of the scsi tree got a conflict in:
>
> drivers/scsi/qedf/qedf.h
>
> between commit:
>
> 8673daf4f55bf3b91 ("qedf: Add get_generic_tlv_data handler.")
>
> from the net-next tree and commit:
>
> 4b9b7fabb39b3e9d7 ("scsi: qedf: Improve firmware debug dump handling")
>
> from the scsi 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.
>
> diff --cc drivers/scsi/qedf/qedf.h
> index cabb6af60fb8,2372a40326f8..000000000000
> --- a/drivers/scsi/qedf/qedf.h
> +++ b/drivers/scsi/qedf/qedf.h
> @@@ -501,9 -499,8 +504,10 @@@ extern int qedf_post_io_req(struct qedf
> extern void qedf_process_seq_cleanup_compl(struct qedf_ctx *qedf,
> struct fcoe_cqe *cqe, struct qedf_ioreq *io_req);
> extern int qedf_send_flogi(struct qedf_ctx *qedf);
> +extern void qedf_get_protocol_tlv_data(void *dev, void *data);
> extern void qedf_fp_io_handler(struct work_struct *work);
> +extern void qedf_get_generic_tlv_data(void *dev, struct qed_generic_tlvs *data);
> + extern void qedf_wq_grcdump(struct work_struct *work);
>
> #define FCOE_WORD_TO_BYTE 4
> #define QEDF_MAX_TASK_NUM 0xFFFF
This is now a conflict between the scsi tree and Linus' tree.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support
From: Jason Wang @ 2018-06-08 3:50 UTC (permalink / raw)
To: mst, jasowang; +Cc: kvm, virtualization, netdev, linux-kernel
This feature bit is duplicated with VIRTIO_F_ANY_LAYOUT, this means if
a userpsace want to enable VRITIO_F_ANY_LAYOUT,
VHOST_NET_F_VIRTIO_NET_HDR will be implied too. This is wrong and will
break networking. Fixing this by safely removing
VHOST_NET_F_VIRTIO_NET_HDR support. There should be very few or even
no userspace can use this. Further cleanups could be done for
-net-next for safety.
In the future, we need a vhost dedicated feature set/get ioctl()
instead of reusing virtio ones.
Fixes: 4e9fa50c6ccbe ("vhost: move features to core")
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 986058a..83eef52 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -69,7 +69,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
- (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
(1ULL << VIRTIO_NET_F_MRG_RXBUF) |
(1ULL << VIRTIO_F_IOMMU_PLATFORM)
};
@@ -1255,15 +1254,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
(1ULL << VIRTIO_F_VERSION_1))) ?
sizeof(struct virtio_net_hdr_mrg_rxbuf) :
sizeof(struct virtio_net_hdr);
- if (features & (1 << VHOST_NET_F_VIRTIO_NET_HDR)) {
- /* vhost provides vnet_hdr */
- vhost_hlen = hdr_len;
- sock_hlen = 0;
- } else {
- /* socket provides vnet_hdr */
- vhost_hlen = 0;
- sock_hlen = hdr_len;
- }
+
+ /* socket provides vnet_hdr */
+ vhost_hlen = 0;
+ sock_hlen = hdr_len;
+
mutex_lock(&n->dev.mutex);
if ((features & (1 << VHOST_F_LOG_ALL)) &&
!vhost_log_access_ok(&n->dev))
--
2.7.4
^ permalink raw reply related
* Re: [v3, 00/10] Support DPAA PTP clock and timestamping
From: Richard Cochran @ 2018-06-08 4:27 UTC (permalink / raw)
To: Yangbo Lu
Cc: netdev, madalin.bucur, Rob Herring, Shawn Guo, David S . Miller,
devicetree, linuxppc-dev, linux-arm-kernel, linux-kernel
In-Reply-To: <20180607092050.46128-1-yangbo.lu@nxp.com>
On Thu, Jun 07, 2018 at 05:20:40PM +0800, Yangbo Lu wrote:
> This patchset is to support DPAA FMAN PTP clock and HW timestamping.
> It had been verified on both ARM platform and PPC platform.
> - The patch #1 to patch #5 are to support DPAA FMAN 1588 timer in
> ptp_qoriq driver.
> - The patch #6 to patch #10 are to add HW timestamping support in
> DPAA ethernet driver.
Right now, net-next is closed for new stuff. You will have to post
the series again after the merge window closes. You can check the
status here:
http://vger.kernel.org/~davem/net-next.html
When you do re-post, you can add my:
Acked-by: Richard Cochran <richardcochran@gmail.com>
^ permalink raw reply
* RE: [v3, 00/10] Support DPAA PTP clock and timestamping
From: Y.b. Lu @ 2018-06-08 4:45 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: <20180608042706.7gfg5p6574ntc2lq@localhost>
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Friday, June 8, 2018 12:27 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: [v3, 00/10] Support DPAA PTP clock and timestamping
>
> On Thu, Jun 07, 2018 at 05:20:40PM +0800, Yangbo Lu wrote:
> > This patchset is to support DPAA FMAN PTP clock and HW timestamping.
> > It had been verified on both ARM platform and PPC platform.
> > - The patch #1 to patch #5 are to support DPAA FMAN 1588 timer in
> > ptp_qoriq driver.
> > - The patch #6 to patch #10 are to add HW timestamping support in
> > DPAA ethernet driver.
>
> Right now, net-next is closed for new stuff. You will have to post the series
> again after the merge window closes. You can check the status here:
>
>
> https://emea01.safelinks.protection.outlook.com/?url=http:%2F%2Fvger.kern
> el.org%2F~davem%2Fnet-next.html&data=02%7C01%7Cyangbo.lu%40nxp.co
> m%7Cbaab0b22e7444386c37008d5ccf81b37%7C686ea1d3bc2b4c6fa92cd99
> c5c301635%7C0%7C0%7C636640288347563742&sdata=jCmNlwoeWA50PV4
> w3lKZ%2Fs4akPjw0VV2OrJ3t4FizJ0%3D&reserved=0
>
> When you do re-post, you can add my:
>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
[Y.b. Lu] Get it. And thanks a lot 😊
^ permalink raw reply
* Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support
From: Michael S. Tsirkin @ 2018-06-08 4:46 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1528429842-22835-1-git-send-email-jasowang@redhat.com>
On Fri, Jun 08, 2018 at 11:50:42AM +0800, Jason Wang wrote:
> This feature bit is duplicated with VIRTIO_F_ANY_LAYOUT, this means if
> a userpsace want to enable VRITIO_F_ANY_LAYOUT,
> VHOST_NET_F_VIRTIO_NET_HDR will be implied too. This is wrong and will
> break networking.
What breaks networking exactly? VHOST_NET supported ANY_LAYOUT
from day one. For this reason it does not need to know about
VRITIO_F_ANY_LAYOUT and we reused the bit for other purposes.
> Fixing this by safely removing
> VHOST_NET_F_VIRTIO_NET_HDR support. There should be very few or even
> no userspace can use this.
Quite possibly, but it is hard to be sure. It seems safer to
maintain it unless there's an actual reason something's broken.
> Further cleanups could be done for
> -net-next for safety.
>
> In the future, we need a vhost dedicated feature set/get ioctl()
> instead of reusing virtio ones.
Not just in the future, we might want to switch iommu
to a sane structure without the 64 bit padding bug
right now.
>
> Fixes: 4e9fa50c6ccbe ("vhost: move features to core")
This tag makes no sense here IMHO. Looks like people are using some tool
that just looks at the earliest version where patch won't apply. The
commit in question just moved some code around.
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/net.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 986058a..83eef52 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -69,7 +69,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
>
> enum {
> VHOST_NET_FEATURES = VHOST_FEATURES |
> - (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
> (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
> (1ULL << VIRTIO_F_IOMMU_PLATFORM)
> };
> @@ -1255,15 +1254,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
> (1ULL << VIRTIO_F_VERSION_1))) ?
> sizeof(struct virtio_net_hdr_mrg_rxbuf) :
> sizeof(struct virtio_net_hdr);
> - if (features & (1 << VHOST_NET_F_VIRTIO_NET_HDR)) {
> - /* vhost provides vnet_hdr */
> - vhost_hlen = hdr_len;
> - sock_hlen = 0;
> - } else {
> - /* socket provides vnet_hdr */
> - vhost_hlen = 0;
> - sock_hlen = hdr_len;
> - }
> +
> + /* socket provides vnet_hdr */
> + vhost_hlen = 0;
> + sock_hlen = hdr_len;
> +
> mutex_lock(&n->dev.mutex);
> if ((features & (1 << VHOST_F_LOG_ALL)) &&
> !vhost_log_access_ok(&n->dev))
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support
From: Jason Wang @ 2018-06-08 5:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20180608074115-mutt-send-email-mst@kernel.org>
On 2018年06月08日 12:46, Michael S. Tsirkin wrote:
> On Fri, Jun 08, 2018 at 11:50:42AM +0800, Jason Wang wrote:
>> This feature bit is duplicated with VIRTIO_F_ANY_LAYOUT, this means if
>> a userpsace want to enable VRITIO_F_ANY_LAYOUT,
>> VHOST_NET_F_VIRTIO_NET_HDR will be implied too. This is wrong and will
>> break networking.
> What breaks networking exactly? VHOST_NET supported ANY_LAYOUT
> from day one. For this reason it does not need to know about
> VRITIO_F_ANY_LAYOUT and we reused the bit for other purposes.
It's the knowledge of vhost_net code it self but not userspace. For
userspace, it should depends on the value of returned by
VHOST_GET_FEATURES. So when userspace can set_features with ANY_LAYOUT,
vhost may think it wants VHOST_NET_F_VIRTIO_NET_HDR.
>
>
>
>> Fixing this by safely removing
>> VHOST_NET_F_VIRTIO_NET_HDR support. There should be very few or even
>> no userspace can use this.
> Quite possibly, but it is hard to be sure. It seems safer to
> maintain it unless there's an actual reason something's broken.
I think not since the feature is negotiated not mandatory?
>
>> Further cleanups could be done for
>> -net-next for safety.
>>
>> In the future, we need a vhost dedicated feature set/get ioctl()
>> instead of reusing virtio ones.
> Not just in the future, we might want to switch iommu
> to a sane structure without the 64 bit padding bug
> right now.
Yes, I hit this bug when introducing V2 of msg IOTLB message.
>
>> Fixes: 4e9fa50c6ccbe ("vhost: move features to core")
> This tag makes no sense here IMHO. Looks like people are using some tool
> that just looks at the earliest version where patch won't apply. The
> commit in question just moved some code around.
Looks not, before this commit, vhost_net won't return ANY_LAYOUT.
Thanks
>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/vhost/net.c | 15 +++++----------
>> 1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>> index 986058a..83eef52 100644
>> --- a/drivers/vhost/net.c
>> +++ b/drivers/vhost/net.c
>> @@ -69,7 +69,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
>>
>> enum {
>> VHOST_NET_FEATURES = VHOST_FEATURES |
>> - (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
>> (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
>> (1ULL << VIRTIO_F_IOMMU_PLATFORM)
>> };
>> @@ -1255,15 +1254,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
>> (1ULL << VIRTIO_F_VERSION_1))) ?
>> sizeof(struct virtio_net_hdr_mrg_rxbuf) :
>> sizeof(struct virtio_net_hdr);
>> - if (features & (1 << VHOST_NET_F_VIRTIO_NET_HDR)) {
>> - /* vhost provides vnet_hdr */
>> - vhost_hlen = hdr_len;
>> - sock_hlen = 0;
>> - } else {
>> - /* socket provides vnet_hdr */
>> - vhost_hlen = 0;
>> - sock_hlen = hdr_len;
>> - }
>> +
>> + /* socket provides vnet_hdr */
>> + vhost_hlen = 0;
>> + sock_hlen = hdr_len;
>> +
>> mutex_lock(&n->dev.mutex);
>> if ((features & (1 << VHOST_F_LOG_ALL)) &&
>> !vhost_log_access_ok(&n->dev))
>> --
>> 2.7.4
^ permalink raw reply
* Re: [PATCH] selftests: bpf: fix urandom_read build issue
From: Y Song @ 2018-06-08 5:08 UTC (permalink / raw)
To: Anders Roxell
Cc: Alexei Starovoitov, Daniel Borkmann, Shuah Khan, netdev, LKML,
linux-kselftest
In-Reply-To: <CADYN=9Ly5UU34CxZFnWfQhNrS8V3gG7=Bdfn9n0h5XphZ8EiVQ@mail.gmail.com>
On Thu, Jun 7, 2018 at 2:43 PM, Anders Roxell <anders.roxell@linaro.org> wrote:
> On 7 June 2018 at 23:17, Y Song <ys114321@gmail.com> wrote:
>> On Thu, Jun 7, 2018 at 12:07 PM, Anders Roxell <anders.roxell@linaro.org> wrote:
>>> On 7 June 2018 at 19:52, Y Song <ys114321@gmail.com> wrote:
>>>> On Thu, Jun 7, 2018 at 3:57 AM, Anders Roxell <anders.roxell@linaro.org> wrote:
>>>>> gcc complains that urandom_read gets built twice.
>>>>>
>>>>> gcc -o tools/testing/selftests/bpf/urandom_read
>>>>> -static urandom_read.c -Wl,--build-id
>>>>> gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../lib/bpf
>>>>> -I../../../../include/generated -I../../../include urandom_read.c
>>>>> urandom_read -lcap -lelf -lrt -lpthread -o
>>>>> tools/testing/selftests/bpf/urandom_read
>>>>> gcc: fatal error: input file
>>>>> ‘tools/testing/selftests/bpf/urandom_read’ is the
>>>>> same as output file
>>>>> compilation terminated.
>>>>> ../lib.mk:110: recipe for target
>>>>> 'tools/testing/selftests/bpf/urandom_read' failed
>>>>
>>>> What is the build/make command to reproduce the above failure?
>>>
>>> make -C tools/testing/selftests
>>
>> Thanks. The patch will break
>> make -C tools/testing/selftests/bpf
>>
>> [yhs@localhost bpf-next]$ make -C tools/testing/selftests/bpf
>> make: Entering directory '/home/yhs/work/bpf-next/tools/testing/selftests/bpf'
>> gcc -o /urandom_read -static urandom_read.c -Wl,--build-id
>> /usr/bin/ld: cannot open output file /urandom_read: Permission denied
>> collect2: error: ld returned 1 exit status
>> make: *** [Makefile:20: /urandom_read] Error 1
>> make: Leaving directory '/home/yhs/work/bpf-next/tools/testing/selftests/bpf'
>> [yhs@localhost bpf-next]$
>
> urgh, I'm sorry, missed that.
>
>>
>> Could you still make the above command work?
>
> $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c
> $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id
>
> That worked both with:
> make -C tools/testing/selftests
> and
> make -C tools/testing/selftests/bpf
>
> for me.
>
> what do you think?
This indeed works. You can submit a revised patch and add my Ack.
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH 2/5] spi: implement companion-spi driver
From: Oleksij Rempel @ 2018-06-08 6:03 UTC (permalink / raw)
To: Jonas Mark (BT-FIR/ENG1)
Cc: Andy Shevchenko, Wolfgang Grandegger, Marc Kleine-Budde,
linux-can@vger.kernel.org, netdev, Linux Kernel Mailing List,
Heiko Schocher, ZHU Yi (BT-FIR/ENG1-Zhu)
In-Reply-To: <be0d6a5f98e840d29f673d976416a4b2@de.bosch.com>
[-- Attachment #1: Type: text/plain, Size: 7658 bytes --]
Hi Mark,
On Thu, Jun 07, 2018 at 02:58:24PM +0000, Jonas Mark (BT-FIR/ENG1) wrote:
> Hello Andy,
>
> Thank you very much for your feedback.
>
> > > + /*TODO: support mutiple packets in one write in future*/
> >
> > Hmm... Either fix this, or remove comment?
>
> Agreed, we will manage ideas for future changes at a different place.
>
> > > + if (copy_from_user(p.data, buf, sizeof(p)) == 0) {
> > > + if (is_can_type(&p))
> > > + return -EINVAL;
> > > + } else {
> > > + dev_info(parent, "copy from user not succeed in one call\n");
> >
> > Shouldn't it return immediately?
>
> Yes, it should. Will be changed.
>
> >
> > > + }
> >
> > > +
> > > + error = qm_io_txq_in(&priv->pm.qm, buf, count, &copied);
> >
> > ...what the point to call if we got garbage from user space?
>
> Will be changed with the code above.
>
> > > + if (!error) {
> >
> > The usual pattern is to check for errors first.
>
> Understood, will be changed.
>
> > > + wake_up_interruptible(&priv->wait);
> > > + priv->pm.stats.io_tx++;
> > > + return copied;
> > > + } else {
> > > + priv->pm.stats.io_tx_overflows++;
> > > + }
> > > + return error;
> > > +}
> >
> > > + ... qm_io_rxq_out(&priv->pm.qm, buf, count, &copied);
> >
> > > + ... pm_can_data_tx(&priv->pm, port, prio, cf);
> >
> > Oh, oh, oh.
> >
> > These namespaces are too generic, moreover pm is kinda occupied by
> > power management. You bring a lot of confusion here, I think.
>
> We agree and we started thinking about better names. We will send a proposal.
>
> > > + err = pm_can_get_txq_status(pm, port);
> > > + if (!err) {
> >
> > if (err)
> > return err;
>
> Yes, will be changed.
>
> > > + }
> > > + return err;
> >
> >
> > > + int ret, value;
> > > +
> > > + ret = sscanf(buf, "%d", &value);
> > > + if (ret != 1) {
> >
> > > + }
> >
> > You have to be consistent in your code.
> >
> > I've already noticed
> >
> > err
> > error
> >
> > and now
> >
> > ret
> >
> > Choose one and stick with it.
>
> Yes, will be changed.
>
> > Also check your entire patch series' code for consistency.
>
> We will have a look.
>
> > > +static DEVICE_ATTR(dump_packets, S_IRUGO | S_IWUSR,
> > > + show_dump_packets, store_dump_packets);
> >
> > We have helpers, like DEVICE_ATTR_RW().
>
> Will be changed.
>
> > > + ret = snprintf(buf + pos, PAGE_SIZE - pos,
> > > + "\ntx: %u, rx: %u, err: %u\n\n",
> > > + total,
> > > + priv->pm.stats.can_rx_overflows[i],
> > > + priv->pm.stats.can_err_overflows[i]);
> >
> > Please, avoid leading '\n'.
>
> We think we will stick to the existing. Otherweise we would have to
> insert another pair of sprint() and pos += ret. Is it worth that?
>
> >
> > > + gpio_set_value(priv->cs_gpios, priv->cs_gpios_assert);
> >
> > > + gpio_set_value(priv->cs_gpios, !priv->cs_gpios_assert);
> >
> > Can you switch to GPIO descriptor API?
>
> Yes, we are working on it.
>
> > > + unsigned int count = READY_POLL_US / READY_POLL_US_GRAN;
> > > + while (count--) {
> >
> > For counting like this better to have
> > do {
> > } while (--count);
> >
> > The rationale, reader at first glance will know that the loop will
> > iterate at least once.
>
> Agreed, will be changed.
>
> > > + if (slave_is_not_busy(priv))
> > > + return 0;
> > > +
> >
> > > + udelay(READY_POLL_US_GRAN);
> >
> > Should it be atomic?
> > Can it use read_poll_* type of helpers instead?
>
> Yes, it shall be atomic because we need to reduce the latency at
> detecting the de-assertion of the busy signal. We accept that this will
> cost CPU time.
>
> In case the Companion itself is very busy and does not reply quickly we
> are have second polling loop below which sleeps longer and is non-atomic.
I can confirm, this make huge impact on protocol performance. And this
protocol is not really the speed runner.
> > Above comments applicable to entire code you have.
>
> We will look at it.
>
> > > +static void companion_spi_cpu_to_be32(char *buf)
> > > +{
> > > + u32 *buf32 = (u32*)buf;
> > > + int i;
> > > +
> > > + for (i = 0; i < (BCP_PACKET_SIZE / sizeof(u32)); i++, buf32++)
> > > + *buf32 = cpu_to_be32(*buf32);
> > > +}
> >
> > This entire function should be replaced by one of the helpers from
> > include/linux/byteorder/generic.h
> >
> > I guess cpu_to_be32_array() is a right one.
>
> Correct. We are testing the driver against 4.14 and that function is not
> available there. It will be changed later.
>
> > > +static void companion_spi_be32_to_cpu(char *buf)
> > > +{
> > > + u32 *buf32 = (u32*)buf;
> > > + int i;
> > > +
> > > + for (i = 0; i < (BCP_PACKET_SIZE / sizeof(u32)); i++, buf32++)
> > > + *buf32 = be32_to_cpu(*buf32);
> > > +}
> >
> > Ditto.
> >
> > Recommendation: check your code against existing helpers.
>
> Yes, every kernel release brings new helpers. We will have to learn how
> to keep track of the additions.
>
> > > + p = (const struct companion_packet*)transfer->tx_buf;
> >
> > > + companion_spi_cpu_to_be32((char*)transfer->tx_buf);
> >
> > If tx_buf is type of void * all these castings are redundant.
>
> The type is const void*. So the first cast is superfluous, the second
> is not.
>
> > Also looking at the function, did you consider to use whatever SPI
> > core provides, like CS handling, messages handling and so on?
>
> SPI CS has to be done manually in our case because we need to wait
> until the Companion signals that it is ready for the transfer.
>
> Do you have concrete proposals regarding messages handling?
you can send dummy message to set CS.
+ struct spi_transfer t = {
+ .len = 0,
+ .cs_change = 1,
+ };
+ /* send dummy to trigger CS */
+ reinit_completion(&priv->fc_complete);
+ spi_sync_locked(spi, &m);
see my ancient unfinished code:
https://patchwork.kernel.org/patch/9418511/
In general, this part of the code, can be provided as separate driver
which should be called as "SPI 5wire protocol". 3 wires for data, 1 -
chip select, 1 - busy state. Since, the slave cant fit to normal SPI
requirements, and can't be ready within predictable time, busy signal is
needed. Providing this as separate driver or part of SPI framework
should reduce the code for many different drivers.
The actual datagram on top of your spi companion should go to
separate driver. There are similar (almost identical designs)
can :+
char:+
dw: +
gpio:+--->some_multi_end_mux_protocol--->spi_5wire_protocol->spi--->
In my knowledge, only data format on top of spi_5wire_protocol is
different. See my notes for similar topics:
https://github.com/olerem/icc
https://github.com/olerem/spi-5wire
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2 0/2] net: phy: improve PHY suspend/resume
From: Heiner Kallweit @ 2018-06-08 6:09 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org
In-Reply-To: <ae4c30a6-7724-a02c-189a-1ff8768528ee@gmail.com>
On 05.06.2018 21:39, Heiner Kallweit wrote:
> On 02.06.2018 22:27, Heiner Kallweit wrote:
>> On 01.06.2018 02:10, Andrew Lunn wrote:
>>>> Configuring the different WoL options isn't handled by writing to
>>>> the PHY registers but by writing to chip / MAC registers.
>>>> Therefore phy_suspend() isn't able to figure out whether WoL is
>>>> enabled or not. Only the parent has the full picture.
>>>
>>> Hi Heiner
>>>
>>> I think you need to look at your different runtime PM domains. If i
>>> understand the code right, you runtime suspend if there is no
>>> link. But for this to work correctly, your PHY needs to keep working.
>>> You also cannot assume all accesses to the PHY go via the MAC. Some
>>> calls will go direct to the PHY, and they can trigger MDIO bus
>>> accesses. So i think you need two runtime PM domains. MAC and MDIO
>>> bus. Maybe just the pll? An MDIO bus is a device, so it can have its
>>> on PM callbacks. It is not clear what you need to resume in order to
>>> make MDIO work.
>>>
>> Thanks for your comments!
>> The actual problem is quite small: I get an error at MDIO suspend,
>> the PHY however is suspended later by the driver's suspend callback
>> anyway. Because the problem is small I'm somewhat reluctant to
>> consider bigger changes like introducing different PM domains.
>>
>> Primary reason for the error is that the network chip is in PCI D3hot
>> at that moment. In addition to that for some of the chips supported by
>> the driver also MDIO-relevant PLL's might be disabled.
>>
>> By the way:
>> When checking PM handling for PHY/MDIO I stumbled across something
>> that can be improved IMO, I'll send a patch for your review.
>>
> I experimented a little and with adding Runtime PM to MDIO bus and
> PHY device I can make it work:
> PHY runtime-resumes before entering suspend and resumes its parent
> (MDIO bus) which then resumes its parent (PCI device).
> However this needs quite some code and is hard to read / understand
> w/o reading through this mail thread.
>
> And in general I still have doubts this is the right way. Let's
> consider the following scenario:
>
> A network driver configures WoL in its suspend callback
> (incl. setting the device to wakeup-enabled).
> The suspend callback of the PHY is called before this and therefore
> has no clue that WoL will be configured a little later, with the
> consequence that it will do an unsolicited power-down.
> The network driver then has to detect this and power-up the PHY again.
> This doesn't seem to make much sense and still my best idea is to
> establish a mechanism that a device can state: I orchestrate PM
> of my children.
>
There's one more way to deal with the issue, an empty dev_pm_domain.
We could do:
struct dev_pm_domain empty = { .ops = NULL };
phydev->mdio.dev.pm_domain = empty;
This overrides the device_type pm ops, however I wouldn't necessarily
consider it an elegant solution. Do you have an opinion on that?
I also checked device links, however this doesn't seem to be the right
concept in our case.
> Heiner
>
>>> It might also help if you do the phy_connect in .ndo_open and
>>> disconnect in .ndo_stop. This is a common pattern in drivers. But some
>>> also do it is probe and remove.
>>>
>> Thanks for the hint. I will move phy_connect_direct accordingly.
>>
>>> Andrew
>>>
>> Heiner
>>
>
^ permalink raw reply
* Re: [PATCH ipsec] vti6: fix PMTU caching and reporting on xmit
From: Steffen Klassert @ 2018-06-08 6:19 UTC (permalink / raw)
To: Eyal Birger; +Cc: netdev, herbert, davem
In-Reply-To: <1528355462-17871-1-git-send-email-eyal.birger@gmail.com>
On Thu, Jun 07, 2018 at 10:11:02AM +0300, Eyal Birger wrote:
> When setting the skb->dst before doing the MTU check, the route PMTU
> caching and reporting is done on the new dst which is about to be
> released.
>
> Instead, PMTU handling should be done using the original dst.
>
> This is aligned with IPv4 VTI.
>
> Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
> Fixes: ccd740cbc6 ("vti6: Add pmtu handling to vti6_xmit.")
Patch applied, thanks for catching this Eyal!
^ permalink raw reply
* Re: [PATCH] selftests: bpf: fix urandom_read build issue
From: Anders Roxell @ 2018-06-08 6:45 UTC (permalink / raw)
To: Y Song
Cc: Alexei Starovoitov, Daniel Borkmann, Shuah Khan, netdev, LKML,
linux-kselftest
In-Reply-To: <CAH3MdRWaVKrYbR14wjn6YjurcHvOFe-=t6g0Me=9=g+GRXqeeg@mail.gmail.com>
On 8 June 2018 at 07:08, Y Song <ys114321@gmail.com> wrote:
> On Thu, Jun 7, 2018 at 2:43 PM, Anders Roxell <anders.roxell@linaro.org> wrote:
>> On 7 June 2018 at 23:17, Y Song <ys114321@gmail.com> wrote:
>>> On Thu, Jun 7, 2018 at 12:07 PM, Anders Roxell <anders.roxell@linaro.org> wrote:
>>>> On 7 June 2018 at 19:52, Y Song <ys114321@gmail.com> wrote:
>>>>> On Thu, Jun 7, 2018 at 3:57 AM, Anders Roxell <anders.roxell@linaro.org> wrote:
>>>>>> gcc complains that urandom_read gets built twice.
>>>>>>
>>>>>> gcc -o tools/testing/selftests/bpf/urandom_read
>>>>>> -static urandom_read.c -Wl,--build-id
>>>>>> gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../lib/bpf
>>>>>> -I../../../../include/generated -I../../../include urandom_read.c
>>>>>> urandom_read -lcap -lelf -lrt -lpthread -o
>>>>>> tools/testing/selftests/bpf/urandom_read
>>>>>> gcc: fatal error: input file
>>>>>> ‘tools/testing/selftests/bpf/urandom_read’ is the
>>>>>> same as output file
>>>>>> compilation terminated.
>>>>>> ../lib.mk:110: recipe for target
>>>>>> 'tools/testing/selftests/bpf/urandom_read' failed
>>>>>
>>>>> What is the build/make command to reproduce the above failure?
>>>>
>>>> make -C tools/testing/selftests
>>>
>>> Thanks. The patch will break
>>> make -C tools/testing/selftests/bpf
>>>
>>> [yhs@localhost bpf-next]$ make -C tools/testing/selftests/bpf
>>> make: Entering directory '/home/yhs/work/bpf-next/tools/testing/selftests/bpf'
>>> gcc -o /urandom_read -static urandom_read.c -Wl,--build-id
>>> /usr/bin/ld: cannot open output file /urandom_read: Permission denied
>>> collect2: error: ld returned 1 exit status
>>> make: *** [Makefile:20: /urandom_read] Error 1
>>> make: Leaving directory '/home/yhs/work/bpf-next/tools/testing/selftests/bpf'
>>> [yhs@localhost bpf-next]$
>>
>> urgh, I'm sorry, missed that.
>>
>>>
>>> Could you still make the above command work?
>>
>> $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c
>> $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id
>>
>> That worked both with:
>> make -C tools/testing/selftests
>> and
>> make -C tools/testing/selftests/bpf
>>
>> for me.
>>
>> what do you think?
>
> This indeed works. You can submit a revised patch and add my Ack.
> Acked-by: Yonghong Song <yhs@fb.com>
Thank you for your time reviewing this.
I will send that out shortly.
Cheers,
Anders
^ permalink raw reply
* [PATCH v2] selftests: bpf: fix urandom_read build issue
From: Anders Roxell @ 2018-06-08 6:51 UTC (permalink / raw)
To: ast, daniel, ys114321, shuah
Cc: netdev, linux-kernel, linux-kselftest, Anders Roxell
In-Reply-To: <CADYN=9JEi2=YcC0KD555atZFiJNbP8Y9O0UME+63iTpSSd=ttg@mail.gmail.com>
gcc complains that urandom_read gets built twice.
gcc -o tools/testing/selftests/bpf/urandom_read
-static urandom_read.c -Wl,--build-id
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../lib/bpf
-I../../../../include/generated -I../../../include urandom_read.c
urandom_read -lcap -lelf -lrt -lpthread -o
tools/testing/selftests/bpf/urandom_read
gcc: fatal error: input file
‘tools/testing/selftests/bpf/urandom_read’ is the
same as output file
compilation terminated.
../lib.mk:110: recipe for target
'tools/testing/selftests/bpf/urandom_read' failed
To fix this issue remove the urandom_read target and so target
TEST_CUSTOM_PROGS gets used.
Fixes: 81f77fd0deeb ("bpf: add selftest for stackmap with BPF_F_STACK_BUILD_ID")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Acked-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/Makefile | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 607ed8729c06..7a6214e9ae58 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -16,9 +16,7 @@ LDLIBS += -lcap -lelf -lrt -lpthread
TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
all: $(TEST_CUSTOM_PROGS)
-$(TEST_CUSTOM_PROGS): urandom_read
-
-urandom_read: urandom_read.c
+$(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c
$(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id
# Order correspond to 'make run_tests' order
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 2/5] spi: implement companion-spi driver
From: Jonas Mark (BT-FIR/ENG1) @ 2018-06-08 6:56 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andy Shevchenko, Wolfgang Grandegger, Marc Kleine-Budde,
linux-can@vger.kernel.org, netdev, Linux Kernel Mailing List,
Heiko Schocher, ZHU Yi (BT-FIR/ENG1-Zhu),
Jonas Mark (BT-FIR/ENG1)
Hi Oleksij,
> > > > + if (slave_is_not_busy(priv))
> > > > + return 0;
> > > > +
> > >
> > > > + udelay(READY_POLL_US_GRAN);
> > >
> > > Should it be atomic?
> > > Can it use read_poll_* type of helpers instead?
> >
> > Yes, it shall be atomic because we need to reduce the latency at
> > detecting the de-assertion of the busy signal. We accept that this will
> > cost CPU time.
> >
> > In case the Companion itself is very busy and does not reply quickly we
> > are have second polling loop below which sleeps longer and is non-atomic.
>
> I can confirm, this make huge impact on protocol performance. And this
> protocol is not really the speed runner.
The challenge is that the protocol is synchronous and without back to
back transfers.
> you can send dummy message to set CS.
> + struct spi_transfer t = {
> + .len = 0,
> + .cs_change = 1,
> + };
>
> + /* send dummy to trigger CS */
> + reinit_completion(&priv->fc_complete);
> + spi_sync_locked(spi, &m);
>
> see my ancient unfinished code:
> https://patchwork.kernel.org/patch/9418511/
We will check it out.
> In general, this part of the code, can be provided as separate driver
> which should be called as "SPI 5wire protocol". 3 wires for data, 1 -
> chip select, 1 - busy state. Since, the slave cant fit to normal SPI
> requirements, and can't be ready within predictable time, busy signal is
> needed. Providing this as separate driver or part of SPI framework
> should reduce the code for many different drivers.
>
> The actual datagram on top of your spi companion should go to
> separate driver. There are similar (almost identical designs)
>
> can :+
> char:+
> dw: +
> gpio:+--->some_multi_end_mux_protocol--->spi_5wire_protocol->spi--->
>
> In my knowledge, only data format on top of spi_5wire_protocol is
> different. See my notes for similar topics:
> https://github.com/olerem/icc
> https://github.com/olerem/spi-5wire
With 5-wire protocol you are referencing to CLK, MISO, MOSI, CS (all
standard SPI signals) and an extra BUSY signal. What we implemented is
a 6-wire protocol. There is an additional REQUEST line where the SPI
slave requests a transfer. It is like a level triggered interrupt
request.
Yes, for making it more generic the code in drivers/spi/companion
could be split into a generic 6-wire protocol driver and a multiplexing
protocol on top of it.
How does a slave signal that it has data to be picked up with the
5-wire protocol?
Greetings,
Mark
Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | www.boschsecurity.com
Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Geschäftsführung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster
^ permalink raw reply
* Re: [PATCH 2/5] spi: implement companion-spi driver
From: Oleksij Rempel @ 2018-06-08 7:05 UTC (permalink / raw)
To: Jonas Mark (BT-FIR/ENG1)
Cc: Andy Shevchenko, Wolfgang Grandegger, Marc Kleine-Budde,
linux-can@vger.kernel.org, netdev, Linux Kernel Mailing List,
Heiko Schocher, ZHU Yi (BT-FIR/ENG1-Zhu)
In-Reply-To: <bb9540ed90e944c7a8e27287e6df25d5@de.bosch.com>
[-- Attachment #1: Type: text/plain, Size: 3228 bytes --]
On Fri, Jun 08, 2018 at 06:56:23AM +0000, Jonas Mark (BT-FIR/ENG1) wrote:
> Hi Oleksij,
>
> > > > > + if (slave_is_not_busy(priv))
> > > > > + return 0;
> > > > > +
> > > >
> > > > > + udelay(READY_POLL_US_GRAN);
> > > >
> > > > Should it be atomic?
> > > > Can it use read_poll_* type of helpers instead?
> > >
> > > Yes, it shall be atomic because we need to reduce the latency at
> > > detecting the de-assertion of the busy signal. We accept that this will
> > > cost CPU time.
> > >
> > > In case the Companion itself is very busy and does not reply quickly we
> > > are have second polling loop below which sleeps longer and is non-atomic.
> >
> > I can confirm, this make huge impact on protocol performance. And this
> > protocol is not really the speed runner.
>
> The challenge is that the protocol is synchronous and without back to
> back transfers.
>
> > you can send dummy message to set CS.
> > + struct spi_transfer t = {
> > + .len = 0,
> > + .cs_change = 1,
> > + };
> >
> > + /* send dummy to trigger CS */
> > + reinit_completion(&priv->fc_complete);
> > + spi_sync_locked(spi, &m);
> >
> > see my ancient unfinished code:
> > https://patchwork.kernel.org/patch/9418511/
>
> We will check it out.
>
> > In general, this part of the code, can be provided as separate driver
> > which should be called as "SPI 5wire protocol". 3 wires for data, 1 -
> > chip select, 1 - busy state. Since, the slave cant fit to normal SPI
> > requirements, and can't be ready within predictable time, busy signal is
> > needed. Providing this as separate driver or part of SPI framework
> > should reduce the code for many different drivers.
> >
> > The actual datagram on top of your spi companion should go to
> > separate driver. There are similar (almost identical designs)
> >
> > can :+
> > char:+
> > dw: +
> > gpio:+--->some_multi_end_mux_protocol--->spi_5wire_protocol->spi--->
> >
> > In my knowledge, only data format on top of spi_5wire_protocol is
> > different. See my notes for similar topics:
> > https://github.com/olerem/icc
> > https://github.com/olerem/spi-5wire
>
> With 5-wire protocol you are referencing to CLK, MISO, MOSI, CS (all
> standard SPI signals) and an extra BUSY signal. What we implemented is
> a 6-wire protocol. There is an additional REQUEST line where the SPI
> slave requests a transfer. It is like a level triggered interrupt
> request.
>
> Yes, for making it more generic the code in drivers/spi/companion
> could be split into a generic 6-wire protocol driver and a multiplexing
> protocol on top of it.
>
> How does a slave signal that it has data to be picked up with the
> 5-wire protocol?
The request and busy signals are multiplexed to one wire. So, it should
be easy to implement both, 5 and 6 wire protocol in one driver.
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net v2] net/sched: act_simple: fix parsing of TCA_DEF_DATA
From: Simon Horman @ 2018-06-08 7:05 UTC (permalink / raw)
To: Davide Caratti
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller, netdev
In-Reply-To: <473a0039fa5c28284ebb5928eb894674cc65b925.1528426801.git.dcaratti@redhat.com>
On Fri, Jun 08, 2018 at 05:02:31AM +0200, Davide Caratti wrote:
> use nla_strlcpy() to avoid copying data beyond the length of TCA_DEF_DATA
> netlink attribute, in case it is less than SIMP_MAX_DATA and it does not
> end with '\0' character.
>
> v2: fix errors in the commit message, thanks Hangbin Liu
>
> Fixes: fa1b1cff3d06 ("net_cls_act: Make act_simple use of netlink policy.")
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
^ permalink raw reply
* [PATCH net,stable] cdc_ncm: avoid padding beyond end of skb
From: Bjørn Mork @ 2018-06-08 7:15 UTC (permalink / raw)
To: netdev
Cc: linux-usb,
Горбешко Богдан,
Dennis Wassenberg, Bjørn Mork, Enrico Mioso
Commit 4a0e3e989d66 ("cdc_ncm: Add support for moving NDP to end
of NCM frame") added logic to reserve space for the NDP at the
end of the NTB/skb. This reservation did not take the final
alignment of the NDP into account, causing us to reserve too
little space. Additionally the padding prior to NDP addition did
not ensure there was enough space for the NDP.
The NTB/skb with the NDP appended would then exceed the configured
max size. This caused the final padding of the NTB to use a
negative count, padding to almost INT_MAX, and resulting in:
[60103.825970] BUG: unable to handle kernel paging request at ffff9641f2004000
[60103.825998] IP: __memset+0x24/0x30
[60103.826001] PGD a6a06067 P4D a6a06067 PUD 4f65a063 PMD 72003063 PTE 0
[60103.826013] Oops: 0002 [#1] SMP NOPTI
[60103.826018] Modules linked in: (removed(
[60103.826158] CPU: 0 PID: 5990 Comm: Chrome_DevTools Tainted: G O 4.14.0-3-amd64 #1 Debian 4.14.17-1
[60103.826162] Hardware name: LENOVO 20081 BIOS 41CN28WW(V2.04) 05/03/2012
[60103.826166] task: ffff964193484fc0 task.stack: ffffb2890137c000
[60103.826171] RIP: 0010:__memset+0x24/0x30
[60103.826174] RSP: 0000:ffff964316c03b68 EFLAGS: 00010216
[60103.826178] RAX: 0000000000000000 RBX: 00000000fffffffd RCX: 000000001ffa5000
[60103.826181] RDX: 0000000000000005 RSI: 0000000000000000 RDI: ffff9641f2003ffc
[60103.826184] RBP: ffff964192f6c800 R08: 00000000304d434e R09: ffff9641f1d2c004
[60103.826187] R10: 0000000000000002 R11: 00000000000005ae R12: ffff9642e6957a80
[60103.826190] R13: ffff964282ff2ee8 R14: 000000000000000d R15: ffff9642e4843900
[60103.826194] FS: 00007f395aaf6700(0000) GS:ffff964316c00000(0000) knlGS:0000000000000000
[60103.826197] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[60103.826200] CR2: ffff9641f2004000 CR3: 0000000013b0c000 CR4: 00000000000006f0
[60103.826204] Call Trace:
[60103.826212] <IRQ>
[60103.826225] cdc_ncm_fill_tx_frame+0x5e3/0x740 [cdc_ncm]
[60103.826236] cdc_ncm_tx_fixup+0x57/0x70 [cdc_ncm]
[60103.826246] usbnet_start_xmit+0x5d/0x710 [usbnet]
[60103.826254] ? netif_skb_features+0x119/0x250
[60103.826259] dev_hard_start_xmit+0xa1/0x200
[60103.826267] sch_direct_xmit+0xf2/0x1b0
[60103.826273] __dev_queue_xmit+0x5e3/0x7c0
[60103.826280] ? ip_finish_output2+0x263/0x3c0
[60103.826284] ip_finish_output2+0x263/0x3c0
[60103.826289] ? ip_output+0x6c/0xe0
[60103.826293] ip_output+0x6c/0xe0
[60103.826298] ? ip_forward_options+0x1a0/0x1a0
[60103.826303] tcp_transmit_skb+0x516/0x9b0
[60103.826309] tcp_write_xmit+0x1aa/0xee0
[60103.826313] ? sch_direct_xmit+0x71/0x1b0
[60103.826318] tcp_tasklet_func+0x177/0x180
[60103.826325] tasklet_action+0x5f/0x110
[60103.826332] __do_softirq+0xde/0x2b3
[60103.826337] irq_exit+0xae/0xb0
[60103.826342] do_IRQ+0x81/0xd0
[60103.826347] common_interrupt+0x98/0x98
[60103.826351] </IRQ>
[60103.826355] RIP: 0033:0x7f397bdf2282
[60103.826358] RSP: 002b:00007f395aaf57d8 EFLAGS: 00000206 ORIG_RAX: ffffffffffffff6e
[60103.826362] RAX: 0000000000000000 RBX: 00002f07bc6d0900 RCX: 00007f39752d7fe7
[60103.826365] RDX: 0000000000000022 RSI: 0000000000000147 RDI: 00002f07baea02c0
[60103.826368] RBP: 0000000000000001 R08: 0000000000000000 R09: 0000000000000000
[60103.826371] R10: 00000000ffffffff R11: 0000000000000000 R12: 00002f07baea02c0
[60103.826373] R13: 00002f07bba227a0 R14: 00002f07bc6d090c R15: 0000000000000000
[60103.826377] Code: 90 90 90 90 90 90 90 0f 1f 44 00 00 49 89 f9 48 89 d1 83
e2 07 48 c1 e9 03 40 0f b6 f6 48 b8 01 01 01 01 01 01 01 01 48 0f af c6 <f3> 48
ab 89 d1 f3 aa 4c 89 c8 c3 90 49 89 f9 40 88 f0 48 89 d1
[60103.826442] RIP: __memset+0x24/0x30 RSP: ffff964316c03b68
[60103.826444] CR2: ffff9641f2004000
Commit e1069bbfcf3b ("net: cdc_ncm: Reduce memory use when kernel
memory low") made this bug much more likely to trigger by reducing
the NTB size under memory pressure.
Link: https://bugs.debian.org/893393
Reported-by: Горбешко Богдан <bodqhrohro@gmail.com>
Reported-and-tested-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
Cc: Enrico Mioso <mrkiko.rs@gmail.com>
Fixes: 4a0e3e989d66 ("cdc_ncm: Add support for moving NDP to end of NCM frame")
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
Big thanks to Dennis for the observation that this crash depended on
FLAG_SEND_ZLP not being set. This made it possible to pinpoint where
the problem was.
drivers/net/usb/cdc_ncm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 90d07ed224d5..b0e8b9613054 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1124,7 +1124,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
* accordingly. Otherwise, we should check here.
*/
if (ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END)
- delayed_ndp_size = ctx->max_ndp_size;
+ delayed_ndp_size = ALIGN(ctx->max_ndp_size, ctx->tx_ndp_modulus);
else
delayed_ndp_size = 0;
@@ -1285,7 +1285,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
/* If requested, put NDP at end of frame. */
if (ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END) {
nth16 = (struct usb_cdc_ncm_nth16 *)skb_out->data;
- cdc_ncm_align_tail(skb_out, ctx->tx_ndp_modulus, 0, ctx->tx_curr_size);
+ cdc_ncm_align_tail(skb_out, ctx->tx_ndp_modulus, 0, ctx->tx_curr_size - ctx->max_ndp_size);
nth16->wNdpIndex = cpu_to_le16(skb_out->len);
skb_put_data(skb_out, ctx->delayed_ndp16, ctx->max_ndp_size);
--
2.11.0
^ permalink raw reply related
* RE: [iproute2 1/1] tipc: TIPC_NLA_LINK_NAME value pass on nesting entry TIPC_NLA_LINK
From: Jon Maloy @ 2018-06-08 7:26 UTC (permalink / raw)
To: Hoang Huu Le, netdev@vger.kernel.org,
tipc-discussion@lists.sourceforge.net, maloy@donjonn.com,
ying.xue@windriver.com
In-Reply-To: <1528424368-3768-1-git-send-email-hoang.h.le@dektech.com.au>
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
> -----Original Message-----
> From: Hoang Le <hoang.h.le@dektech.com.au>
> Sent: Friday, 08 June, 2018 04:19
> To: netdev@vger.kernel.org; tipc-discussion@lists.sourceforge.net; Jon
> Maloy <jon.maloy@ericsson.com>; maloy@donjonn.com;
> ying.xue@windriver.com
> Subject: [iproute2 1/1] tipc: TIPC_NLA_LINK_NAME value pass on nesting
> entry TIPC_NLA_LINK
>
> In the commit 94f6a80 on next-net, TIPC_NLA_LINK_NAME attribute should
> be retrieved and validated via TIPC_NLA_LINK nesting entry in
> tipc_nl_node_get_link().
> According to that commit, TIPC_NLA_LINK_NAME value passing via tipc link
> get command must follow above hierachy.
>
> Acked-by: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
> ---
> tipc/link.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tipc/link.c b/tipc/link.c
> index 02f14aadefa6..a2d7c0016bc1 100644
> --- a/tipc/link.c
> +++ b/tipc/link.c
> @@ -97,6 +97,7 @@ static int cmd_link_get_prop(struct nlmsghdr *nlh,
> const struct cmd *cmd, {
> int prop;
> char buf[MNL_SOCKET_BUFFER_SIZE];
> + struct nlattr *attrs;
> struct opt *opt;
> struct opt opts[] = {
> { "link", OPT_KEYVAL, NULL },
> @@ -131,7 +132,9 @@ static int cmd_link_get_prop(struct nlmsghdr *nlh,
> const struct cmd *cmd,
> fprintf(stderr, "error, missing link\n");
> return -EINVAL;
> }
> + attrs = mnl_attr_nest_start(nlh, TIPC_NLA_LINK);
> mnl_attr_put_strz(nlh, TIPC_NLA_LINK_NAME, opt->val);
> + mnl_attr_nest_end(nlh, attrs);
>
> return msg_doit(nlh, link_get_cb, &prop); }
> --
> 2.1.4
^ permalink raw reply
* Re: [PATCH v4 9/9] net-next: New ax88796 platform driver for Amiga X-Surf 100 Zorro board (m68k)
From: Michael Schmitz @ 2018-06-08 7:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: netdev, Andrew Lunn, Finn Thain, Florian Fainelli, Linux/m68k,
Michael Karcher, Michael Karcher
In-Reply-To: <CAMuHMdWsibY4h5zwBC4qs8ZLq_1-627qjpeDLGzVjaGjG_A4PA@mail.gmail.com>
Hi Geert,
Am 08.06.2018 um 02:36 schrieb Geert Uytterhoeven:
> Hi Michael,
>
> On Thu, Apr 19, 2018 at 4:05 AM, Michael Schmitz <schmitzmic@gmail.com> wrote:
>> From: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de>
>>
>> Add platform device driver to populate the ax88796 platform data from
>> information provided by the XSurf100 zorro device driver. The ax88796
>> module will be loaded through this module's probe function.
>>
>> Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de>
>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>
> This is now commit 861928f4e60e826c ("net-next: New ax88796 platform
> driver for Amiga X-Surf 100 Zorro board (m68k)").
>
>> --- /dev/null
>> +++ b/drivers/net/ethernet/8390/xsurf100.c
>
>> +#define __NS8390_init ax_NS8390_init
>
> [...]
>
>> +#include "lib8390.c"
>
> drivers/net/ethernet/8390/lib8390.c:202: warning: ‘__ei_open’ defined
> but not used
> drivers/net/ethernet/8390/lib8390.c:231: warning: ‘__ei_close’ defined
> but not used
> drivers/net/ethernet/8390/lib8390.c:255: warning: ‘__ei_tx_timeout’
> defined but not used
> drivers/net/ethernet/8390/lib8390.c:302: warning: ‘__ei_start_xmit’
> defined but not used
> drivers/net/ethernet/8390/lib8390.c:510: warning: ‘__ei_poll’ defined
> but not used
> drivers/net/ethernet/8390/lib8390.c:851: warning: ‘__ei_get_stats’
> defined but not used
> drivers/net/ethernet/8390/lib8390.c:951: warning:
> ‘__ei_set_multicast_list’ defined but not used
> drivers/net/ethernet/8390/lib8390.c:989: warning:
> ‘____alloc_ei_netdev’ defined but not used
>
Yep, these have been a little annoying...
> So I was wondering: why is this file included, as XSURF100 selects AX88796,
> while ax88796.c includes lib8390.c anyway?
>
> Apparently lib8390.c is included for register definitions (provided by
> 8390.h, and can easily be fixed), and for the __NS8390_init()
> implementation, called below.
Mostly the latter.
>
>> +static void xs100_block_output(struct net_device *dev, int count,
>> + const unsigned char *buf, const int start_page)
>> +{
>
> [...]
>
>> + while ((ei_inb(nic_base + EN0_ISR) & ENISR_RDC) == 0) {
>> + if (jiffies - dma_start > 2 * HZ / 100) { /* 20ms */
>> + netdev_warn(dev, "timeout waiting for Tx RDC.\n");
>> + ei_local->reset_8390(dev);
>> + ax_NS8390_init(dev, 1);
>> + break;
>> + }
>> + }
>> +
>> + ei_outb(ENISR_RDC, nic_base + EN0_ISR); /* Ack intr. */
>> + ei_local->dmaing &= ~0x01;
>> +}
>
> Can we get rid of the inclusion of lib8390.c, and the related warnings?
> Perhaps ax88796.c can export its ax_NS8390_init(), iff the implementation
> is identical? Or is there a better solution?
__NS8390_init() is declared static in lib8390.c, and we'd have to change
that. I don't think that will fly. Adding a wrapper to ax88796.c that
gets exported to the xsurf100 module might be an option - I'll see what
I can come up with.
Cheers,
Michael
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
^ permalink raw reply
* Re: WARNING in smc_unhash_sk
From: Dmitry Vyukov @ 2018-06-08 7:53 UTC (permalink / raw)
To: Davide Caratti
Cc: syzbot, David Miller, LKML, linux-s390, netdev, syzkaller-bugs,
ubraun
In-Reply-To: <1519734229.2699.6.camel@redhat.com>
On Tue, Feb 27, 2018 at 1:23 PM, Davide Caratti <dcaratti@redhat.com> wrote:
> On Fri, 2018-02-23 at 07:59 -0800, syzbot wrote:
>> Hello,
>>
>> syzbot hit the following crash on upstream commit
>> af3e79d29555b97dd096e2f8e36a0f50213808a8 (Tue Feb 20 18:05:02 2018 +0000)
>> Merge tag 'leds_for-4.16-rc3' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds
>>
>> So far this crash happened 27 times on
>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/master,
>> net-next, upstream.
>> C reproducer is attached.
>> syzkaller reproducer is attached.
>> Raw console output is attached.
>> compiler: gcc (GCC) 7.1.1 20170620
>> .config is attached.
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+3a0748c8f2f210c0ef9b@syzkaller.appspotmail.com
>> It will help syzbot understand when the bug is fixed. See footer for
>> details.
>> If you forward the report, please keep this part and the footer.
>>
>> WARNING: CPU: 1 PID: 9921 at ./include/net/sock.h:638 sk_del_node_init
>> include/net/sock.h:638 [inline]
>> WARNING: CPU: 1 PID: 9921 at ./include/net/sock.h:638
>> smc_unhash_sk+0x335/0x450 net/smc/af_smc.c:90
>> Kernel panic - not syncing: panic_on_warn set ...
>>
>> CPU: 1 PID: 9921 Comm: syzkaller089677 Not tainted 4.16.0-rc2+ #324
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>> Google 01/01/2011
>> Call Trace:
>> __dump_stack lib/dump_stack.c:17 [inline]
>> dump_stack+0x194/0x24d lib/dump_stack.c:53
>> panic+0x1e4/0x41c kernel/panic.c:183
>> __warn+0x1dc/0x200 kernel/panic.c:547
>> report_bug+0x211/0x2d0 lib/bug.c:184
>> fixup_bug.part.11+0x37/0x80 arch/x86/kernel/traps.c:178
>> fixup_bug arch/x86/kernel/traps.c:247 [inline]
>> do_error_trap+0x2d7/0x3e0 arch/x86/kernel/traps.c:296
>> do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
>> invalid_op+0x58/0x80 arch/x86/entry/entry_64.S:957
>> RIP: 0010:sk_del_node_init include/net/sock.h:638 [inline]
>> RIP: 0010:smc_unhash_sk+0x335/0x450 net/smc/af_smc.c:90
>> RSP: 0018:ffff8801b639f198 EFLAGS: 00010293
>> RAX: ffff8801b684a340 RBX: 1ffff10036c73e37 RCX: ffffffff85a3f1f5
>> RDX: 0000000000000000 RSI: dffffc0000000000 RDI: 1ffff10036c73e3b
>> RBP: ffff8801b639f280 R08: dffffc0000000000 R09: 0000000000000004
>> R10: ffff8801b639f050 R11: 0000000000000004 R12: ffff8801b639f258
>> R13: ffffffff87669b40 R14: ffff8801c08d57c0 R15: 1ffff10036c73e3b
>> smc_release+0x321/0x580 net/smc/af_smc.c:148
>> sock_release+0x8d/0x1e0 net/socket.c:595
>> sock_close+0x16/0x20 net/socket.c:1149
>> __fput+0x327/0x7e0 fs/file_table.c:209
>> ____fput+0x15/0x20 fs/file_table.c:243
>> task_work_run+0x199/0x270 kernel/task_work.c:113
>> exit_task_work include/linux/task_work.h:22 [inline]
>> do_exit+0x9bb/0x1ad0 kernel/exit.c:865
>> do_group_exit+0x149/0x400 kernel/exit.c:968
>> get_signal+0x73a/0x16d0 kernel/signal.c:2469
>> do_signal+0x90/0x1e90 arch/x86/kernel/signal.c:809
>> exit_to_usermode_loop+0x258/0x2f0 arch/x86/entry/common.c:162
>> prepare_exit_to_usermode arch/x86/entry/common.c:196 [inline]
>> syscall_return_slowpath arch/x86/entry/common.c:265 [inline]
>> do_syscall_64+0x6e5/0x940 arch/x86/entry/common.c:292
>> entry_SYSCALL_64_after_hwframe+0x42/0xb7
>> RIP: 0033:0x44cad9
>> RSP: 002b:00007fbf24687ce8 EFLAGS: 00000246 ORIG_RAX: 0000000000000007
>> RAX: 0000000000000001 RBX: 0000000000700024 RCX: 000000000044cad9
>> RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000020000080
>> RBP: 0000000000700020 R08: 0000000000000000 R09: 0000000000000000
>> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
>> R13: 000000000080ef3f R14: 00007fbf246889c0 R15: 0000000000000005
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Kernel Offset: disabled
>> Rebooting in 86400 seconds..
>>
>
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
Hi Davide,
Any progress on this?
This is syzbot top crasher at the moment with 86000+ machines crashed:
https://syzkaller.appspot.com/bug?id=004b0f7b61d4901cbfecfc33de7996e8cbe0a278
^ permalink raw reply
* Re: INFO: task hung in ip6gre_exit_batch_net
From: Dmitry Vyukov @ 2018-06-08 8:18 UTC (permalink / raw)
To: Kirill Tkhai
Cc: syzbot, Christian Brauner, David Miller, David Ahern,
Florian Westphal, Jiri Benc, LKML, Xin Long, mschiffer, netdev,
syzkaller-bugs, Vladislav Yasevich
In-Reply-To: <6b14e8b9-c335-dd46-98a6-7f58a624fcea@virtuozzo.com>
On Thu, Jun 7, 2018 at 9:59 PM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> On 07.06.2018 22:03, Dmitry Vyukov wrote:
>> On Thu, Jun 7, 2018 at 8:54 PM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>>>>>>> Hi, Dmirty!
>>>>>>>
>>>>>>> On 04.06.2018 18:22, Dmitry Vyukov wrote:
>>>>>>>> On Mon, Jun 4, 2018 at 5:03 PM, syzbot
>>>>>>>> <syzbot+bf78a74f82c1cf19069e@syzkaller.appspotmail.com> wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> syzbot found the following crash on:
>>>>>>>>>
>>>>>>>>> HEAD commit: bc2dbc5420e8 Merge branch 'akpm' (patches from Andrew)
>>>>>>>>> git tree: upstream
>>>>>>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=164e42b7800000
>>>>>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=982e2df1b9e60b02
>>>>>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=bf78a74f82c1cf19069e
>>>>>>>>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>>>>>>>>
>>>>>>>>> Unfortunately, I don't have any reproducer for this crash yet.
>>>>>>>>>
>>>>>>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>>>>>>> Reported-by: syzbot+bf78a74f82c1cf19069e@syzkaller.appspotmail.com
>>>>>>>>
>>>>>>>> Another hang on rtnl lock:
>>>>>>>>
>>>>>>>> #syz dup: INFO: task hung in netdev_run_todo
>>>>>>>>
>>>>>>>> May be related to "unregister_netdevice: waiting for DEV to become free":
>>>>>>>> https://syzkaller.appspot.com/bug?id=1a97a5bd119fd97995f752819fd87840ab9479a9
>>>>>>
>>>>>> netdev_wait_allrefs does not hold rtnl lock during waiting, so it must
>>>>>> be something different.
>>>>>>
>>>>>>
>>>>>>>> Any other explanations for massive hangs on rtnl lock for minutes?
>>>>>>>
>>>>>>> To exclude the situation, when a task exists with rtnl_mutex held:
>>>>>>>
>>>>>>> would the pr_warn() from print_held_locks_bug() be included in the console output
>>>>>>> if they appear?
>>>>>>
>>>>>> Yes, everything containing "WARNING:" is detected as bug.
>>>>>
>>>>> OK, then dead task not releasing the lock is excluded.
>>>>>
>>>>> One more assumption: someone corrupted memory around rtnl_mutex and it looks like locked.
>>>>> (I track lockdep "(rtnl_mutex){+.+.}" prints in initial message as "nobody owns rtnl_mutex").
>>>>> There may help a crash dump of the VM.
>>>>
>>>> I can't find any legend for these +'s and .'s, but {+.+.} is present
>>>> in large amounts in just any task hung report for different mutexes,
>>>> so I would not expect that it means corruption.
>>>>
>>>> Are dozens of known corruptions that syzkaller can trigger. But
>>>> usually they are reliably caught by KASAN. If any of them would lead
>>>> to silent memory corruption, we would got dozens of assorted crashes
>>>> throughout the kernel. We've seen that at some points, but not
>>>> recently. So I would assume that memory is not corrupted in all these
>>>> cases:
>>>> https://syzkaller.appspot.com/bug?id=2503c576cabb08d41812e732b390141f01a59545
>>>
>>> This BUG clarifies the {+.+.}:
>>>
>>> 4 locks held by kworker/0:145/381:
>>> #0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] work_static include/linux/workqueue.h:198 [inline]
>>> #0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] set_work_data kernel/workqueue.c:619 [inline]
>>> #0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] set_work_pool_and_clear_pending kernel/workqueue.c:646 [inline]
>>> #0: ((wq_completion)"hwsim_wq"){+.+.}, at: [<000000003f9487f0>] process_one_work+0xb12/0x1bb0 kernel/workqueue.c:2084
>>> #1: ((work_completion)(&data->destroy_work)){+.+.}, at: [<00000000bbdd2115>] process_one_work+0xb89/0x1bb0 kernel/workqueue.c:2088
>>> #2: (rtnl_mutex){+.+.}, at: [<000000009c9d14f8>] rtnl_lock+0x17/0x20 net/core/rtnetlink.c:74
>>> #3: (rcu_sched_state.exp_mutex){+.+.}, at: [<000000001ba1a807>] exp_funnel_lock kernel/rcu/tree_exp.h:272 [inline]
>>> #3: (rcu_sched_state.exp_mutex){+.+.}, at: [<000000001ba1a807>] _synchronize_rcu_expedited.constprop.72+0x9fa/0xac0 kernel/rcu/tree_exp.h:596
>>>
>>> There we have rtnl_mutex locked and the {..} is like above. It's definitely locked
>>> since there is one more lock after it.
>>>
>>> This BUG happen because of there are many rtnl_mutex waiters while owner
>>> is synchronizing RCU. Rather clear for me in comparison to the topic's hung.
>>
>>
>> You mean that it's not hanged, but rather needs more than 2 minutes to
>> complete, right?
>
> Yeah, I think, this is possible. I've seen the situations like that.
> Let synchronize_rcu_expedited() is executed for X seconds. Then,
> it's need just 120/x calls of "this function under rtnl_mutex" to make
> a soft lockup of someone else who wants the mutex too.
>
> Also, despite the CFS is fair scheduler, in case of the calls are
> made from workqueue, every work will cause sleep. So, every work
> will be executed in separate worker task. Every worker will haved its
> own time slice. This increases the probability these tasks will
> take cpu time before the task in the header of the hang.
OK, let's stick with this theory for now. Looking at the crash frequencies here:
https://syzkaller.appspot.com/bug?id=2503c576cabb08d41812e732b390141f01a59545
I can actually believe that this is just flakes due to too slow execution.
I've noted that we need either reduce load and/or increase timeouts:
https://github.com/google/syzkaller/issues/516#issuecomment-395685629
Let's keep duping new such reports onto "INFO: task hung in
netdev_run_todo" so that they are all collected at a single location.
Thanks for help
^ 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