* [PATCH V8 2/3] bfin_can: introduce ioremap to comply to archs with MMU
From: Aaron Wu @ 2015-02-10 9:40 UTC (permalink / raw)
To: wg, mkl, linux-can, netdev, linux-kernel, adi-buildroot-devel; +Cc: Aaron Wu
In-Reply-To: <1423561237-11043-1-git-send-email-Aaron.wu@analog.com>
Blackfin was built without MMU, old driver code access the IO space by
physical address, introduce the ioremap approach to be compitable with
the common style supporting MMU enabled arch.
Signed-off-by: Aaron Wu <Aaron.wu@analog.com>
---
drivers/net/can/bfin_can.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c
index 0a5eff4..259d09a 100644
--- a/drivers/net/can/bfin_can.c
+++ b/drivers/net/can/bfin_can.c
@@ -551,16 +551,10 @@ static int bfin_can_probe(struct platform_device *pdev)
goto exit;
}
- if (!request_mem_region(res_mem->start, resource_size(res_mem),
- dev_name(&pdev->dev))) {
- err = -EBUSY;
- goto exit;
- }
-
/* request peripheral pins */
err = peripheral_request_list(pdata, dev_name(&pdev->dev));
if (err)
- goto exit_mem_release;
+ goto exit;
dev = alloc_bfin_candev();
if (!dev) {
@@ -569,7 +563,13 @@ static int bfin_can_probe(struct platform_device *pdev)
}
priv = netdev_priv(dev);
- priv->membase = (void __iomem *)res_mem->start;
+
+ priv->membase = devm_ioremap_resource(&pdev->dev, res_mem);
+ if (IS_ERR(priv->membase)) {
+ err = PTR_ERR(priv->membase);
+ goto exit_peri_pin_free;
+ }
+
priv->rx_irq = rx_irq->start;
priv->tx_irq = tx_irq->start;
priv->err_irq = err_irq->start;
@@ -601,8 +601,6 @@ exit_candev_free:
free_candev(dev);
exit_peri_pin_free:
peripheral_free_list(pdata);
-exit_mem_release:
- release_mem_region(res_mem->start, resource_size(res_mem));
exit:
return err;
}
@@ -611,15 +609,11 @@ static int bfin_can_remove(struct platform_device *pdev)
{
struct net_device *dev = platform_get_drvdata(pdev);
struct bfin_can_priv *priv = netdev_priv(dev);
- struct resource *res;
bfin_can_set_reset_mode(dev);
unregister_candev(dev);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- release_mem_region(res->start, resource_size(res));
-
peripheral_free_list(priv->pin_list);
free_candev(dev);
--
1.7.9.5
^ permalink raw reply related
* [PATCH V8 3/3] bfin_can: Merge header file from arch dependent location
From: Aaron Wu @ 2015-02-10 9:40 UTC (permalink / raw)
To: wg, mkl, linux-can, netdev, linux-kernel, adi-buildroot-devel; +Cc: Aaron Wu
In-Reply-To: <1423561237-11043-1-git-send-email-Aaron.wu@analog.com>
Header file was in arch dependent location arch/blackfin/include/asm/bfin_can.h,
Now move and merge the useful contents of header file into driver code, note
the original header file is reserved for full registers set access test by other
code so it survives.
Signed-off-by: Aaron Wu <Aaron.wu@analog.com>
---
drivers/net/can/bfin_can.c | 110 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 109 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c
index 259d09a..bb1d480 100644
--- a/drivers/net/can/bfin_can.c
+++ b/drivers/net/can/bfin_can.c
@@ -20,13 +20,121 @@
#include <linux/can/dev.h>
#include <linux/can/error.h>
-#include <asm/bfin_can.h>
#include <asm/portmux.h>
#define DRV_NAME "bfin_can"
#define BFIN_CAN_TIMEOUT 100
#define TX_ECHO_SKB_MAX 1
+/* transmit and receive channels */
+#define TRANSMIT_CHL 24
+#define RECEIVE_STD_CHL 0
+#define RECEIVE_EXT_CHL 4
+#define RECEIVE_RTR_CHL 8
+#define RECEIVE_EXT_RTR_CHL 12
+#define MAX_CHL_NUMBER 32
+
+/* All Blackfin system MMRs are padded to 32bits even if the register
+ * itself is only 16bits. So use a helper macro to streamline this
+*/
+#define __BFP(m) u16 m; u16 __pad_##m
+
+/* bfin can registers layout */
+struct bfin_can_mask_regs {
+ __BFP(aml);
+ __BFP(amh);
+};
+
+struct bfin_can_channel_regs {
+ /* data[0,2,4,6] -> data{0,1,2,3} while data[1,3,5,7] is padding */
+ u16 data[8];
+ __BFP(dlc);
+ __BFP(tsv);
+ __BFP(id0);
+ __BFP(id1);
+};
+
+struct bfin_can_regs {
+ /* global control and status registers */
+ __BFP(mc1); /* offset 0x00 */
+ __BFP(md1); /* offset 0x04 */
+ __BFP(trs1); /* offset 0x08 */
+ __BFP(trr1); /* offset 0x0c */
+ __BFP(ta1); /* offset 0x10 */
+ __BFP(aa1); /* offset 0x14 */
+ __BFP(rmp1); /* offset 0x18 */
+ __BFP(rml1); /* offset 0x1c */
+ __BFP(mbtif1); /* offset 0x20 */
+ __BFP(mbrif1); /* offset 0x24 */
+ __BFP(mbim1); /* offset 0x28 */
+ __BFP(rfh1); /* offset 0x2c */
+ __BFP(opss1); /* offset 0x30 */
+ u32 __pad1[3];
+ __BFP(mc2); /* offset 0x40 */
+ __BFP(md2); /* offset 0x44 */
+ __BFP(trs2); /* offset 0x48 */
+ __BFP(trr2); /* offset 0x4c */
+ __BFP(ta2); /* offset 0x50 */
+ __BFP(aa2); /* offset 0x54 */
+ __BFP(rmp2); /* offset 0x58 */
+ __BFP(rml2); /* offset 0x5c */
+ __BFP(mbtif2); /* offset 0x60 */
+ __BFP(mbrif2); /* offset 0x64 */
+ __BFP(mbim2); /* offset 0x68 */
+ __BFP(rfh2); /* offset 0x6c */
+ __BFP(opss2); /* offset 0x70 */
+ u32 __pad2[3];
+ __BFP(clock); /* offset 0x80 */
+ __BFP(timing); /* offset 0x84 */
+ __BFP(debug); /* offset 0x88 */
+ __BFP(status); /* offset 0x8c */
+ __BFP(cec); /* offset 0x90 */
+ __BFP(gis); /* offset 0x94 */
+ __BFP(gim); /* offset 0x98 */
+ __BFP(gif); /* offset 0x9c */
+ __BFP(control); /* offset 0xa0 */
+ __BFP(intr); /* offset 0xa4 */
+ __BFP(version); /* offset 0xa8 */
+ __BFP(mbtd); /* offset 0xac */
+ __BFP(ewr); /* offset 0xb0 */
+ __BFP(esr); /* offset 0xb4 */
+ u32 __pad3[2];
+ __BFP(ucreg); /* offset 0xc0 */
+ __BFP(uccnt); /* offset 0xc4 */
+ __BFP(ucrc); /* offset 0xc8 */
+ __BFP(uccnf); /* offset 0xcc */
+ u32 __pad4[1];
+ __BFP(version2); /* offset 0xd4 */
+ u32 __pad5[10];
+
+ /* channel(mailbox) mask and message registers */
+ struct bfin_can_mask_regs msk[MAX_CHL_NUMBER]; /* offset 0x100 */
+ struct bfin_can_channel_regs chl[MAX_CHL_NUMBER]; /* offset 0x200 */
+};
+
+#undef __BFP
+
+#define SRS 0x0001 /* Software Reset */
+#define SER 0x0008 /* Stuff Error */
+#define BOIM 0x0008 /* Enable Bus Off Interrupt */
+#define CCR 0x0080 /* CAN Configuration Mode Request */
+#define CCA 0x0080 /* Configuration Mode Acknowledge */
+#define SAM 0x0080 /* Sampling */
+#define AME 0x8000 /* Acceptance Mask Enable */
+#define RMLIM 0x0080 /* Enable RX Message Lost Interrupt */
+#define RMLIS 0x0080 /* RX Message Lost IRQ Status */
+#define RTR 0x4000 /* Remote Frame Transmission Request */
+#define BOIS 0x0008 /* Bus Off IRQ Status */
+#define IDE 0x2000 /* Identifier Extension */
+#define EPIS 0x0004 /* Error-Passive Mode IRQ Status */
+#define EPIM 0x0004 /* Enable Error-Passive Mode Interrupt */
+#define EWTIS 0x0001 /* TX Error Count IRQ Status */
+#define EWRIS 0x0002 /* RX Error Count IRQ Status */
+#define BEF 0x0040 /* Bit Error Flag */
+#define FER 0x0080 /* Form Error Flag */
+#define SMR 0x0020 /* Sleep Mode Request */
+#define SMACK 0x0008 /* Sleep Mode Acknowledge */
+
/*
* bfin can private data
*/
--
1.7.9.5
^ permalink raw reply related
* Re: ipv6: oops in datagram.c line 260
From: Steffen Klassert @ 2015-02-10 9:57 UTC (permalink / raw)
To: Chris Ruehl; +Cc: Hannes Frederic Sowa, netdev, davem
In-Reply-To: <54D46F50.7020104@gtsys.com.hk>
On Fri, Feb 06, 2015 at 03:37:52PM +0800, Chris Ruehl wrote:
> Hi Steffen,
>
> server is up for 6 days no problems any more.
> Please apply the patch!
>
> Thank you very much
> Chris
>
> Tested-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
Now applied to the ipsec tree. Thanks a lot for testing Chris!
^ permalink raw reply
* Re: [PATCH V8 1/3] bfin_can: rewrite the blackfin style of read/write to common ones
From: Marc Kleine-Budde @ 2015-02-10 10:05 UTC (permalink / raw)
To: Aaron Wu, wg, linux-can, netdev, linux-kernel,
adi-buildroot-devel
In-Reply-To: <1423561237-11043-1-git-send-email-Aaron.wu@analog.com>
[-- Attachment #1: Type: text/plain, Size: 456 bytes --]
On 02/10/2015 10:40 AM, Aaron Wu wrote:
> Replace the blackfin arch dependent style of bfin_read/bfin_write with
> common readw/writew
Applied to can-next.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v5 net-next 1/6] virtio_ring: fix virtqueue_enable_cb() when only 1 buffers were pending
From: Michael S. Tsirkin @ 2015-02-10 10:18 UTC (permalink / raw)
To: Rusty Russell; +Cc: pagupta, netdev, linux-kernel, virtualization
In-Reply-To: <878ug6y3uv.fsf@rustcorp.com.au>
On Tue, Feb 10, 2015 at 11:33:52AM +1030, Rusty Russell wrote:
> Jason Wang <jasowang@redhat.com> writes:
> > We currently does:
> >
> > bufs = (avail->idx - last_used_idx) * 3 / 4;
> >
> > This is ok now since we only try to enable the delayed callbacks when
> > the queue is about to be full. This may not work well when there is
> > only one pending buffer in the virtqueue (this may be the case after
> > tx interrupt was enabled). Since virtqueue_enable_cb() will return
> > false which may cause unnecessary triggering of napis. This patch
> > correct this by only calculate the four thirds when bufs is not one.
>
> I mildly prefer to avoid the branch, by changing the calculation like
> so:
>
> /* Set bufs >= 1, even if there's only one pending buffer */
> bufs = (bufs + 1) * 3 / 4;
Or bus * 3/4 + 1
> But it's not clear to me how much this happens. I'm happy with the
> patch though, as currently virtqueue_enable_cb_delayed() is the same
> as virtqueue_enable_cb() if there's only been one buffer added.
>
> Cheers,
> Rusty.
But isn't this by design?
The documentation says:
* This re-enables callbacks but hints to the other side to delay
* interrupts until most of the available buffers have been processed;
Clearly, this implies that when there's one buffer it must behave
exactly the same.
So I'm not very happy - this changes the meaning of the API without
updating the documentation.
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> > drivers/virtio/virtio_ring.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 00ec6b3..545fed5 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -636,7 +636,10 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
> > * entry. Always do both to keep code simple. */
> > vq->vring.avail->flags &= cpu_to_virtio16(_vq->vdev, ~VRING_AVAIL_F_NO_INTERRUPT);
> > /* TODO: tune this threshold */
> > - bufs = (u16)(virtio16_to_cpu(_vq->vdev, vq->vring.avail->idx) - vq->last_used_idx) * 3 / 4;
> > + bufs = (u16)(virtio16_to_cpu(_vq->vdev, vq->vring.avail->idx) -
> > + vq->last_used_idx);
> > + if (bufs != 1)
> > + bufs = bufs * 3 / 4;
> > vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev, vq->last_used_idx + bufs);
> > virtio_mb(vq->weak_barriers);
> > if (unlikely((u16)(virtio16_to_cpu(_vq->vdev, vq->vring.used->idx) - vq->last_used_idx) > bufs)) {
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > Virtualization mailing list
> > Virtualization@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH RFC v5 net-next 2/6] virtio_ring: try to disable event index callbacks in virtqueue_disable_cb()
From: Michael S. Tsirkin @ 2015-02-10 10:24 UTC (permalink / raw)
To: Jason Wang; +Cc: pagupta, netdev, linux-kernel, virtualization
In-Reply-To: <1423471165-34243-3-git-send-email-jasowang@redhat.com>
On Mon, Feb 09, 2015 at 03:39:21AM -0500, Jason Wang wrote:
> Currently, we do nothing to prevent the callbacks in
> virtqueue_disable_cb() when event index is used. This may cause
> spurious interrupts which may damage the performance. This patch tries
> to publish avail event as the used even to prevent the callbacks.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
I'm surprised that this ever happens though.
Normally we call this after getting an interrupt, and
interrupts won't trigger again until the rings wraps around.
When I tested this, touching an extra cache line was more
expensive.
Does this really reduce number of interrupts?
Could you pls share some numbers with and without this patch?
> ---
> drivers/virtio/virtio_ring.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 545fed5..e9ffbfb 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -539,6 +539,8 @@ void virtqueue_disable_cb(struct virtqueue *_vq)
> struct vring_virtqueue *vq = to_vvq(_vq);
>
> vq->vring.avail->flags |= cpu_to_virtio16(_vq->vdev, VRING_AVAIL_F_NO_INTERRUPT);
> + vring_used_event(&vq->vring) = cpu_to_virtio16(_vq->vdev,
> + vq->vring.avail->idx);
Hmm in fact, can't this actually cause an extra interrupt
when avail->idx is completed?
I think that if you really can show disabling interrupts like this helps, you should
set some invalid value like 0xfffff, or move it back to vq->vring.avail->idx - 1.
No?
> }
> EXPORT_SYMBOL_GPL(virtqueue_disable_cb);
>
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH RFC v5 net-next 4/6] virtio-net: add basic interrupt coalescing support
From: Michael S. Tsirkin @ 2015-02-10 10:25 UTC (permalink / raw)
To: Jason Wang; +Cc: pagupta, netdev, linux-kernel, virtualization
In-Reply-To: <1423551090.897.1@smtp.corp.redhat.com>
On Tue, Feb 10, 2015 at 06:59:30AM +0008, Jason Wang wrote:
>
>
> On Tue, Feb 10, 2015 at 9:32 AM, Rusty Russell <rusty@rustcorp.com.au>
> wrote:
> >Jason Wang <jasowang@redhat.com> writes:
> >> This patch enables the interrupt coalescing setting through ethtool.
> >
> >The problem is that there's nothing network specific about interrupt
> >coalescing. I can see other devices wanting exactly the same thing,
> >which means we'd deprecate this in the next virtio standard.
> >
> >I think the right answer is to extend like we did with
> >vring_used_event(), eg:
> >
> >1) Add a new feature VIRTIO_F_RING_COALESCE.
> >2) Add another a 32-bit field after vring_used_event(), eg:
> > #define vring_used_delay(vr) (*(u32 *)((vr)->avail->ring[(vr)->num
> >+ 2]))
>
> Yes. This looks better and we don't even need device specific configuration
> method.
>
> >
> >This loses the ability to coalesce by number of frames, but we can still
> >do number of sg entries, as we do now with used_event, and we could
> >change virtqueue_enable_cb_delayed() to take a precise number if we
> >wanted.
>
> Can we give a device specific meaning for this? For virtio-net, we want to
> expose the coalescing settings through ethtool (tx-frames). And it was
> usually used with a timer, so probably another field after
> vring_used_delay() for this timer interval to trigger the interrupt if no
> new used buffers come after this interval.
I think what Rusty has in mind is precisely sticking the delay
in vring_used_delay.
> >
> >
> >My feeling is that this should be a v1.0-only feature though
> >(eg. feature bit 33).
>
> Yes it should.
>
> >
> >Cheers,
> >Rusty.
> >
> >> Cc: Rusty Russell <rusty@rustcorp.com.au>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >> ---
> >> drivers/net/virtio_net.c | 67
> >>+++++++++++++++++++++++++++++++++++++++++
> >> include/uapi/linux/virtio_net.h | 12 ++++++++
> >> 2 files changed, 79 insertions(+)
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index cc5f5de..2b958fb 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -145,6 +145,11 @@ struct virtnet_info {
> >> /* Budget for polling tx completion */
> >> u32 tx_work_limit;
> >> +
> >> + __u32 rx_coalesce_usecs;
> >> + __u32 rx_max_coalesced_frames;
> >> + __u32 tx_coalesce_usecs;
> >> + __u32 tx_max_coalesced_frames;
> >> };
> >> struct padded_vnet_hdr {
> >> @@ -1404,12 +1409,73 @@ static void virtnet_get_channels(struct
> >>net_device *dev,
> >> channels->other_count = 0;
> >> }
> >> +static int virtnet_set_coalesce(struct net_device *dev,
> >> + struct ethtool_coalesce *ec)
> >> +{
> >> + struct virtnet_info *vi = netdev_priv(dev);
> >> + struct scatterlist sg;
> >> + struct virtio_net_ctrl_coalesce c;
> >> +
> >> + if (!vi->has_cvq ||
> >> + !virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_COALESCE))
> >> + return -EOPNOTSUPP;
> >> + if (vi->rx_coalesce_usecs != ec->rx_coalesce_usecs ||
> >> + vi->rx_max_coalesced_frames != ec->rx_max_coalesced_frames) {
> >> + c.coalesce_usecs = ec->rx_coalesce_usecs;
> >> + c.max_coalesced_frames = ec->rx_max_coalesced_frames;
> >> + sg_init_one(&sg, &c, sizeof(c));
> >> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_COALESCE,
> >> + VIRTIO_NET_CTRL_COALESCE_RX_SET,
> >> + &sg)) {
> >> + dev_warn(&dev->dev, "Fail to set rx coalescing\n");
> >> + return -EINVAL;
> >> + }
> >> + vi->rx_coalesce_usecs = ec->rx_coalesce_usecs;
> >> + vi->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
> >> + }
> >> +
> >> + if (vi->tx_coalesce_usecs != ec->tx_coalesce_usecs ||
> >> + vi->tx_max_coalesced_frames != ec->tx_max_coalesced_frames) {
> >> + c.coalesce_usecs = ec->tx_coalesce_usecs;
> >> + c.max_coalesced_frames = ec->tx_max_coalesced_frames;
> >> + sg_init_one(&sg, &c, sizeof(c));
> >> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_COALESCE,
> >> + VIRTIO_NET_CTRL_COALESCE_TX_SET,
> >> + &sg)) {
> >> + dev_warn(&dev->dev, "Fail to set tx coalescing\n");
> >> + return -EINVAL;
> >> + }
> >> + vi->tx_coalesce_usecs = ec->tx_coalesce_usecs;
> >> + vi->tx_max_coalesced_frames = ec->tx_max_coalesced_frames;
> >> + }
> >> +
> >> + vi->tx_work_limit = ec->tx_max_coalesced_frames_irq;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int virtnet_get_coalesce(struct net_device *dev,
> >> + struct ethtool_coalesce *ec)
> >> +{
> >> + struct virtnet_info *vi = netdev_priv(dev);
> >> +
> >> + ec->rx_coalesce_usecs = vi->rx_coalesce_usecs;
> >> + ec->rx_max_coalesced_frames = vi->rx_max_coalesced_frames;
> >> + ec->tx_coalesce_usecs = vi->tx_coalesce_usecs;
> >> + ec->tx_max_coalesced_frames = vi->tx_max_coalesced_frames;
> >> + ec->tx_max_coalesced_frames_irq = vi->tx_work_limit;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static const struct ethtool_ops virtnet_ethtool_ops = {
> >> .get_drvinfo = virtnet_get_drvinfo,
> >> .get_link = ethtool_op_get_link,
> >> .get_ringparam = virtnet_get_ringparam,
> >> .set_channels = virtnet_set_channels,
> >> .get_channels = virtnet_get_channels,
> >> + .set_coalesce = virtnet_set_coalesce,
> >> + .get_coalesce = virtnet_get_coalesce,
> >> };
> >> #define MIN_MTU 68
> >> @@ -2048,6 +2114,7 @@ static unsigned int features[] = {
> >> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> >> VIRTIO_NET_F_CTRL_MAC_ADDR,
> >> VIRTIO_F_ANY_LAYOUT,
> >> + VIRTIO_NET_F_CTRL_COALESCE,
> >> };
> >> static struct virtio_driver virtio_net_driver = {
> >> diff --git a/include/uapi/linux/virtio_net.h
> >>b/include/uapi/linux/virtio_net.h
> >> index b5f1677..332009d 100644
> >> --- a/include/uapi/linux/virtio_net.h
> >> +++ b/include/uapi/linux/virtio_net.h
> >> @@ -34,6 +34,7 @@
> >> /* The feature bitmap for virtio net */
> >> #define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */
> >> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial
> >>csum */
> >> +#define VIRTIO_NET_F_CTRL_COALESCE 3 /* Set coalescing */
> >> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
> >> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> >> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */
> >> @@ -202,4 +203,15 @@ struct virtio_net_ctrl_mq {
> >> #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN 1
> >> #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX 0x8000
> >> +struct virtio_net_ctrl_coalesce {
> >> + __u32 coalesce_usecs;
> >> + __u32 max_coalesced_frames;
> >> +};
> >> +
> >> +#define VIRTIO_NET_CTRL_COALESCE 6
> >> + #define VIRTIO_NET_CTRL_COALESCE_TX_SET 0
> >> + #define VIRTIO_NET_CTRL_COALESCE_TX_GET 1
> >> + #define VIRTIO_NET_CTRL_COALESCE_RX_SET 2
> >> + #define VIRTIO_NET_CTRL_COALESCE_RX_GET 3
> >> +
> >> #endif /* _LINUX_VIRTIO_NET_H */
> >> -- 1.8.3.1
> >--
> >To unsubscribe from this list: send the line "unsubscribe netdev" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Michal Kazior @ 2015-02-10 10:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: Neal Cardwell, linux-wireless, Network Development, Eyal Perry
In-Reply-To: <1423494690.31870.189.camel@edumazet-glaptop2.roam.corp.google.com>
On 9 February 2015 at 16:11, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2015-02-09 at 14:47 +0100, Michal Kazior wrote:
[...]
> This is not what I suggested.
>
> If you test this on any other network device, you'll have
> sk->sk_tx_completion_delay_us == 0
>
> amount = 0 * (sk->sk_pacing_rate >> 10); --> 0
> limit = max(2 * skb->truesize, amount >> 10); --> 2 * skb->truesize
You're right. Sorry for mixing up.
> So non TSO/GSO NIC will not be able to queue more than 2 MSS (one MSS
> per skb)
>
> Then if you store only the last tx completion, you have the possibility
> of having a last packet of a train (say a retransmit) to make it very
> low.
>
> Ideally the formula would be in TCP something very fast to compute :
>
> amount = (sk->sk_pacing_rate >> 10) + sk->tx_completion_delay_cushion;
> limit = max(2 * skb->truesize, amount);
> limit = min_t(u32, limit, sysctl_tcp_limit_output_bytes);
>
> So a 'problematic' driver would have to do the math (64 bit maths) like
> this :
>
>
> sk->tx_completion_delay_cushion = ewma_tx_delay * sk->sk_pacing_rate;
Hmm. So I've done like you suggested (hopefully I didn't mix anything
up this time around).
I now get pre-regression performance, ~250mbps on 1 flow, 600mbps on 5
flows (vs 250mbps whatever number of flows).
Michał
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 367e896..a29111c 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
#include <linux/of.h>
+#include <linux/average.h>
#include "core.h"
#include "mac.h"
@@ -1423,6 +1424,7 @@ struct ath10k *ath10k_core_create(size_t
priv_size, struct device *dev,
init_dummy_netdev(&ar->napi_dev);
ieee80211_napi_add(ar->hw, &ar->napi, &ar->napi_dev,
ath10k_core_napi_dummy_poll, 64);
+ ewma_init(&ar->tx_delay_us, 16384, 8);
ret = ath10k_debug_create(ar);
if (ret)
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 3be3a59..34f6d78 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -24,6 +24,7 @@
#include <linux/pci.h>
#include <linux/uuid.h>
#include <linux/time.h>
+#include <linux/average.h>
#include "htt.h"
#include "htc.h"
@@ -82,6 +83,7 @@ struct ath10k_skb_cb {
dma_addr_t paddr;
u8 eid;
u8 vdev_id;
+ ktime_t stamp;
struct {
u8 tid;
@@ -625,6 +627,7 @@ struct ath10k {
struct net_device napi_dev;
struct napi_struct napi;
+ struct ewma tx_delay_us;
#ifdef CONFIG_ATH10K_DEBUGFS
struct ath10k_debug debug;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c
b/drivers/net/wireless/ath/ath10k/mac.c
index 15e47f4..5efb2a7 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2620,6 +2620,7 @@ static void ath10k_tx(struct ieee80211_hw *hw,
if (info->flags & IEEE80211_TX_CTL_NO_CCK_RATE)
ath10k_dbg(ar, ATH10K_DBG_MAC,
"IEEE80211_TX_CTL_NO_CCK_RATE\n");
+ ATH10K_SKB_CB(skb)->stamp = ktime_get();
ATH10K_SKB_CB(skb)->htt.is_offchan = false;
ATH10K_SKB_CB(skb)->htt.tid = ath10k_tx_h_get_tid(hdr);
ATH10K_SKB_CB(skb)->vdev_id = ath10k_tx_h_get_vdev_id(ar, vif);
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c
b/drivers/net/wireless/ath/ath10k/txrx.c
index 3f00cec..0f5f0f2 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -15,6 +15,8 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#include <net/sock.h>
+#include <linux/average.h>
#include "core.h"
#include "txrx.h"
#include "htt.h"
@@ -82,6 +84,16 @@ void ath10k_txrx_tx_unref(struct ath10k_htt *htt,
ath10k_report_offchan_tx(htt->ar, msdu);
+ if (msdu->sk) {
+ ewma_add(&ar->tx_delay_us,
+ ktime_to_ns(ktime_sub(ktime_get(), skb_cb->stamp)) /
+ NSEC_PER_USEC);
+
+ ACCESS_ONCE(msdu->sk->sk_tx_completion_delay_cushion) =
+ (ewma_read(&ar->tx_delay_us) *
+ msdu->sk->sk_pacing_rate) >> 20;
+ }
+
info = IEEE80211_SKB_CB(msdu);
memset(&info->status, 0, sizeof(info->status));
trace_ath10k_txrx_tx_unref(ar, tx_done->msdu_id);
diff --git a/include/net/sock.h b/include/net/sock.h
index 2210fec..6772543 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -391,6 +391,7 @@ struct sock {
gfp_t sk_allocation;
u32 sk_pacing_rate; /* bytes per second */
u32 sk_max_pacing_rate;
+ u32 sk_tx_completion_delay_cushion;
netdev_features_t sk_route_caps;
netdev_features_t sk_route_nocaps;
int sk_gso_type;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 65caf8b..526a568 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1996,6 +1996,7 @@ static bool tcp_write_xmit(struct sock *sk,
unsigned int mss_now, int nonagle,
max_segs = tcp_tso_autosize(sk, mss_now);
while ((skb = tcp_send_head(sk))) {
unsigned int limit;
+ unsigned int amount;
tso_segs = tcp_init_tso_segs(sk, skb, mss_now);
BUG_ON(!tso_segs);
@@ -2053,7 +2054,9 @@ static bool tcp_write_xmit(struct sock *sk,
unsigned int mss_now, int nonagle,
* of queued bytes to ensure line rate.
* One example is wifi aggregation (802.11 AMPDU)
*/
- limit = max(2 * skb->truesize, sk->sk_pacing_rate >> 10);
+ amount = (sk->sk_pacing_rate >> 10) +
+ sk->sk_tx_completion_delay_cushion;
+ limit = max(2 * skb->truesize, amount);
limit = min_t(u32, limit, sysctl_tcp_limit_output_bytes);
if (atomic_read(&sk->sk_wmem_alloc) > limit) {
^ permalink raw reply related
* Re: [PATCH] net: qmi_wwan: MC73xx interface 10 is not QMI
From: Aleksander Morgado @ 2015-02-10 10:37 UTC (permalink / raw)
To: Kristian Evensen; +Cc: Bjørn Mork, David Miller, netdev@vger.kernel.org
In-Reply-To: <CAKfDRXhyS0fdonKryTLtr8PcJ60XbaxxZbynR3kfFKcfioB2tQ@mail.gmail.com>
On Tue, Feb 10, 2015 at 10:18 AM, Kristian Evensen
<kristian.evensen@gmail.com> wrote:
>> BTW, regarding the patch... if interface #10 ends up being usable only
>> in some 73xx models, I would still leave it available anyway in the
>> kernel driver. Userspace can always figure out whether the interface
>> is usable or not (e.g. MM does some QMI probing on the interface
>> before flagging it as usable).
>
> I will not fight strongly for this patch. My motivation was mostly to
> clean up, since it looks a bit messy keeping an additional interface +
> cdc-wdm device around which will never be used (and can be avoided).
With ModemManager you can use both WWAN interfaces by creating
independent Bearer objects and getting them connected independently
(e.g. to different APNs). Not sure if anyone will ever do that, but
well.
--
Aleksander
https://aleksander.es
^ permalink raw reply
* Re: [PATCH RFC v5 net-next 4/6] virtio-net: add basic interrupt coalescing support
From: Michael S. Tsirkin @ 2015-02-10 10:40 UTC (permalink / raw)
To: Rusty Russell; +Cc: pagupta, netdev, linux-kernel, virtualization
In-Reply-To: <87386ey2iy.fsf@rustcorp.com.au>
On Tue, Feb 10, 2015 at 12:02:37PM +1030, Rusty Russell wrote:
> Jason Wang <jasowang@redhat.com> writes:
> > This patch enables the interrupt coalescing setting through ethtool.
>
> The problem is that there's nothing network specific about interrupt
> coalescing. I can see other devices wanting exactly the same thing,
> which means we'd deprecate this in the next virtio standard.
>
> I think the right answer is to extend like we did with
> vring_used_event(), eg:
>
> 1) Add a new feature VIRTIO_F_RING_COALESCE.
> 2) Add another a 32-bit field after vring_used_event(), eg:
> #define vring_used_delay(vr) (*(u32 *)((vr)->avail->ring[(vr)->num + 2]))
>
> This loses the ability to coalesce by number of frames, but we can still
> do number of sg entries, as we do now with used_event, and we could
> change virtqueue_enable_cb_delayed() to take a precise number if we
> wanted.
But do we expect delay to be update dynamically?
If not, why not stick it in config space?
> My feeling is that this should be a v1.0-only feature though
> (eg. feature bit 33).
>
> Cheers,
> Rusty.
Yes, e.g. we can't extend config space for legacy virtio pci.
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> > drivers/net/virtio_net.c | 67 +++++++++++++++++++++++++++++++++++++++++
> > include/uapi/linux/virtio_net.h | 12 ++++++++
> > 2 files changed, 79 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index cc5f5de..2b958fb 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -145,6 +145,11 @@ struct virtnet_info {
> >
> > /* Budget for polling tx completion */
> > u32 tx_work_limit;
> > +
> > + __u32 rx_coalesce_usecs;
> > + __u32 rx_max_coalesced_frames;
> > + __u32 tx_coalesce_usecs;
> > + __u32 tx_max_coalesced_frames;
> > };
> >
> > struct padded_vnet_hdr {
> > @@ -1404,12 +1409,73 @@ static void virtnet_get_channels(struct net_device *dev,
> > channels->other_count = 0;
> > }
> >
> > +static int virtnet_set_coalesce(struct net_device *dev,
> > + struct ethtool_coalesce *ec)
> > +{
> > + struct virtnet_info *vi = netdev_priv(dev);
> > + struct scatterlist sg;
> > + struct virtio_net_ctrl_coalesce c;
> > +
> > + if (!vi->has_cvq ||
> > + !virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_COALESCE))
> > + return -EOPNOTSUPP;
> > + if (vi->rx_coalesce_usecs != ec->rx_coalesce_usecs ||
> > + vi->rx_max_coalesced_frames != ec->rx_max_coalesced_frames) {
> > + c.coalesce_usecs = ec->rx_coalesce_usecs;
> > + c.max_coalesced_frames = ec->rx_max_coalesced_frames;
> > + sg_init_one(&sg, &c, sizeof(c));
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_COALESCE,
> > + VIRTIO_NET_CTRL_COALESCE_RX_SET,
> > + &sg)) {
> > + dev_warn(&dev->dev, "Fail to set rx coalescing\n");
> > + return -EINVAL;
> > + }
> > + vi->rx_coalesce_usecs = ec->rx_coalesce_usecs;
> > + vi->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
> > + }
> > +
> > + if (vi->tx_coalesce_usecs != ec->tx_coalesce_usecs ||
> > + vi->tx_max_coalesced_frames != ec->tx_max_coalesced_frames) {
> > + c.coalesce_usecs = ec->tx_coalesce_usecs;
> > + c.max_coalesced_frames = ec->tx_max_coalesced_frames;
> > + sg_init_one(&sg, &c, sizeof(c));
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_COALESCE,
> > + VIRTIO_NET_CTRL_COALESCE_TX_SET,
> > + &sg)) {
> > + dev_warn(&dev->dev, "Fail to set tx coalescing\n");
> > + return -EINVAL;
> > + }
> > + vi->tx_coalesce_usecs = ec->tx_coalesce_usecs;
> > + vi->tx_max_coalesced_frames = ec->tx_max_coalesced_frames;
> > + }
> > +
> > + vi->tx_work_limit = ec->tx_max_coalesced_frames_irq;
> > +
> > + return 0;
> > +}
> > +
> > +static int virtnet_get_coalesce(struct net_device *dev,
> > + struct ethtool_coalesce *ec)
> > +{
> > + struct virtnet_info *vi = netdev_priv(dev);
> > +
> > + ec->rx_coalesce_usecs = vi->rx_coalesce_usecs;
> > + ec->rx_max_coalesced_frames = vi->rx_max_coalesced_frames;
> > + ec->tx_coalesce_usecs = vi->tx_coalesce_usecs;
> > + ec->tx_max_coalesced_frames = vi->tx_max_coalesced_frames;
> > + ec->tx_max_coalesced_frames_irq = vi->tx_work_limit;
> > +
> > + return 0;
> > +}
> > +
> > static const struct ethtool_ops virtnet_ethtool_ops = {
> > .get_drvinfo = virtnet_get_drvinfo,
> > .get_link = ethtool_op_get_link,
> > .get_ringparam = virtnet_get_ringparam,
> > .set_channels = virtnet_set_channels,
> > .get_channels = virtnet_get_channels,
> > + .set_coalesce = virtnet_set_coalesce,
> > + .get_coalesce = virtnet_get_coalesce,
> > };
> >
> > #define MIN_MTU 68
> > @@ -2048,6 +2114,7 @@ static unsigned int features[] = {
> > VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> > VIRTIO_NET_F_CTRL_MAC_ADDR,
> > VIRTIO_F_ANY_LAYOUT,
> > + VIRTIO_NET_F_CTRL_COALESCE,
> > };
> >
> > static struct virtio_driver virtio_net_driver = {
> > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > index b5f1677..332009d 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -34,6 +34,7 @@
> > /* The feature bitmap for virtio net */
> > #define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */
> > #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */
> > +#define VIRTIO_NET_F_CTRL_COALESCE 3 /* Set coalescing */
> > #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
> > #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> > #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */
> > @@ -202,4 +203,15 @@ struct virtio_net_ctrl_mq {
> > #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN 1
> > #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX 0x8000
> >
> > +struct virtio_net_ctrl_coalesce {
> > + __u32 coalesce_usecs;
> > + __u32 max_coalesced_frames;
> > +};
> > +
> > +#define VIRTIO_NET_CTRL_COALESCE 6
> > + #define VIRTIO_NET_CTRL_COALESCE_TX_SET 0
> > + #define VIRTIO_NET_CTRL_COALESCE_TX_GET 1
> > + #define VIRTIO_NET_CTRL_COALESCE_RX_SET 2
> > + #define VIRTIO_NET_CTRL_COALESCE_RX_GET 3
> > +
> > #endif /* _LINUX_VIRTIO_NET_H */
> > --
> > 1.8.3.1
^ permalink raw reply
* Re: [RFC PATCH 00/29] net: VRF support
From: Derek Fawcus @ 2015-02-10 10:43 UTC (permalink / raw)
To: David Ahern
Cc: roopa, netdev, ebiederm, Dinesh Dutt, Vipin Kumar,
Nicolas Dichtel, hannes
In-Reply-To: <54D424BD.2090402@gmail.com>
On Thu, Feb 05, 2015 at 07:19:41pm -0700, David Ahern wrote:
> On 2/5/15 4:12 PM, roopa wrote:
> >Wondering if you have thought about some of the the below cases in your
> >approach to vrfs ?.
> >- Leaking routes from one vrf to another
>
> Can you give me an example of what you mean by this?
I believe the following sort of thing is being referred to:
http://packetlife.net/blog/2010/mar/29/inter-vrf-routing-vrf-lite/
http://packetlife.net/blog/2013/sep/26/vrf-export-maps/
https://rekrowten.wordpress.com/2012/09/24/route-leak-between-vrfs-with-import-maps-and-export-maps/
I google search for "bgp import OR export" brings a bunch of hits.
DF
^ permalink raw reply
* Re: [PATCH] net: qmi_wwan: MC73xx interface 10 is not QMI
From: Kristian Evensen @ 2015-02-10 10:53 UTC (permalink / raw)
To: Aleksander Morgado; +Cc: Bjørn Mork, David Miller, netdev@vger.kernel.org
In-Reply-To: <CAAP7ucJMDcbvCw_1B33NOpWOoj4-or86MDQM9rw14Zs8KRUdxQ@mail.gmail.com>
On Tue, Feb 10, 2015 at 11:37 AM, Aleksander Morgado
<aleksander@aleksander.es> wrote:
> With ModemManager you can use both WWAN interfaces by creating
> independent Bearer objects and getting them connected independently
> (e.g. to different APNs). Not sure if anyone will ever do that, but
> well.
Thanks for reminding me of this use case. One scenario where it makes
sense is how IPv4/IPv6 is handled by some mobile broadband operators.
I know of at least one Norwegian ISP that at least used different APNs
for IPv4 and IPv6.
-Kristian
^ permalink raw reply
* linux-next: build failure after merge of the net-next tree
From: Stephen Rothwell @ 2015-02-10 11:02 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Carolyn Wyborny, Jeff Kirsher
[-- Attachment #1: Type: text/plain, Size: 718 bytes --]
Hi all,
After merging the net-next tree, today's linux-next build (i386
allmodconfig and a few others) failed like this:
drivers/net/ethernet/intel/i40e/i40e_txrx.c: In function 'i40e_setup_rx_descriptors':
drivers/net/ethernet/intel/i40e/i40e_txrx.c:1101:3: error: incompatible type for argument 1 of '__seqcount_init'
include/linux/seqlock.h:53:20: note: expected 'struct seqcount_t *' but argument is of type 'seqcount_t'
Caused by commit 638702bd59a3 ("i40e/i40evf: Add call to u64_stats_init
to init").
Discovered after the release, so no fix done.
The indentation of the lines added by that patch is also a bit strange.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] net: qmi_wwan: MC73xx interface 10 is not QMI
From: Bjørn Mork @ 2015-02-10 11:39 UTC (permalink / raw)
To: Kristian Evensen; +Cc: Aleksander Morgado, David Miller, netdev@vger.kernel.org
In-Reply-To: <CAKfDRXhQ5vDUw_0X-N8v9NhTwxo_Osc-JZtAorm0wjw9W5r+LA@mail.gmail.com>
Kristian Evensen <kristian.evensen@gmail.com> writes:
> On Tue, Feb 10, 2015 at 11:37 AM, Aleksander Morgado
> <aleksander@aleksander.es> wrote:
>> With ModemManager you can use both WWAN interfaces by creating
>> independent Bearer objects and getting them connected independently
>> (e.g. to different APNs). Not sure if anyone will ever do that, but
>> well.
>
> Thanks for reminding me of this use case. One scenario where it makes
> sense is how IPv4/IPv6 is handled by some mobile broadband operators.
> I know of at least one Norwegian ISP that at least used different APNs
> for IPv4 and IPv6.
Sure, very useful for testing, although I don't think that they meant it
that way. I believe the assumption is that you get full Internet access
using either APN, with DNS64/NAT64/464XLAT for the IPv6 only one.
Other use cases I can think of:
- dedicated MMS APN
- dedicated voice APN
- VPN APN(s)
all in combination with an Internet access APN. So yes, two independent
bearers are certainly useful. I expect many modems will support more
than one bearer in the near future. The Intel XMM7160 based modems,
like for example the Sierra Wireless EM7345, already support 8(!)
Bjørn
^ permalink raw reply
* Re: bride: IPv6 multicast snooping enhancements
From: Linus Lüssing @ 2015-02-10 11:44 UTC (permalink / raw)
To: Vasily Averin
Cc: Herbert Xu, netdev, bridge, linux-kernel, David S. Miller,
Cong Wang
In-Reply-To: <54D9C4ED.6040601@parallels.com>
Hi Vasily,
On Tue, Feb 10, 2015 at 11:44:29AM +0300, Vasily Averin wrote:
> This patch prevent forwarding of ICMPv6 in bridges,
> so containers/VMs with virtual eth adapters connected in local bridge cannot ping each other via ipv6 (but can do it via ipv4)
If a host wants to receive packets, then it needs to signalize
that via MLD. If your host does not do that, then it is expected
to not receive ICMPv6 echo requests to multicast addresses. An
exception is ff02::1, that should always work.
>
> Could you please clarify, is it expected behavior?
> Do we need to enable multicast routing or multicast_snooping on all local ports on such bridges to enable just ICMPv6?
Nope, you shouldn't. You'd need multicast listeners. You shouldn't
need to play with the bridge settings to fix protocols.
> I believe ICMPv6 is an exception and should not be filtered by multicast spoofing.
Signaling multicast joins is mandatory by the IPv6 standard. If
your protocol/application does not do that, then it seems to me
that the application might be broken.
By the way, which kernel version(s) are you using?
Cheers, Linus
^ permalink raw reply
* Re: [PATCH v3 linux-trace 4/8] samples: bpf: simple tracing example in C
From: Steven Rostedt @ 2015-02-10 12:24 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML,
Linus Torvalds
In-Reply-To: <CAMEtUuzon5LfG7PS9YmuW+0GzYMKehz1Ddk+6tXogZOZYdpb3g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Added Linus because he's the one that would revert changes on breakage.
On Mon, 9 Feb 2015 21:45:21 -0800
Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Mon, Feb 9, 2015 at 9:16 PM, Steven Rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org> wrote:
> > On Mon, 9 Feb 2015 23:08:36 -0500
> > Steven Rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org> wrote:
> >
> >> I don't want to get stuck with pinned kernel data structures again. We
> >> had 4 blank bytes of data for every event, because latency top hard
> >> coded the field. Luckily, the 64 bit / 32 bit interface caused latency
> >> top to have to use the event_parse code to work, and we were able to
> >> remove that field after it was converted.
>
> I think your main point boils down to:
>
> > But I still do not want any hard coded event structures. All access to
> > data from the binary code must be parsed by looking at the event/format
> > files. Otherwise you will lock internals of the kernel as userspace
> > ABI, because eBPF programs will break if those internals change, and
> > that could severely limit progress in the future.
>
> and I completely agree.
>
> the patch 4 is an example. It doesn't mean in any way
> that structs defined here is an ABI.
> To be compatible across kernels the user space must read
> format file as you mentioned in your other reply.
The thing is, this is a sample. Which means it will be cut and pasted
into other programs. If the sample does not follow the way we want
users to use this, then how can we complain if they hard code it as
well?
>
> > I'm wondering if we should label eBPF programs as "modules". That is,
> > they have no guarantee of working from one kernel to the next. They
> > execute in the kernel, thus they are very similar to modules.
> >
> > If we can get Linus to say that eBPF programs are not user space, and
> > that they are treated the same as modules (no internal ABI), then I
> > think we can be a bit more free at what we allow.
>
> I thought we already stated that.
> Here is the quote from perf_event.h:
> * # The RAW record below is opaque data wrt the ABI
> * #
> * # That is, the ABI doesn't make any promises wrt to
> * # the stability of its content, it may vary depending
> * # on event, hardware, kernel version and phase of
> * # the moon.
> * #
> * # In other words, PERF_SAMPLE_RAW contents are not an ABI.
>
> and this example is reading PERF_SAMPLE_RAW events and
> uses locally defined structs to print them for simplicity.
As we found out the hard way with latencytop, comments like this does
not matter. If an application does something like this, it's our fault
if it breaks later. We can't say "hey you were suppose to do it this
way". That argument breaks down even more if our own examples do not
follow the way we want others to do things.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 linux-trace 4/8] samples: bpf: simple tracing example in C
From: Steven Rostedt @ 2015-02-10 12:27 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML,
Linus Torvalds
In-Reply-To: <CAMEtUuyCmO+N2SOTNXfOVnRA59TWsZczPCJBWsB+h0E45NxHPA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, 9 Feb 2015 21:47:42 -0800
Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Mon, Feb 9, 2015 at 9:45 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> > I thought we already stated that.
> > Here is the quote from perf_event.h:
> > * # The RAW record below is opaque data wrt the ABI
> > * #
> > * # That is, the ABI doesn't make any promises wrt to
> > * # the stability of its content, it may vary depending
> > * # on event, hardware, kernel version and phase of
> > * # the moon.
> > * #
> > * # In other words, PERF_SAMPLE_RAW contents are not an ABI.
> >
> > and this example is reading PERF_SAMPLE_RAW events and
> > uses locally defined structs to print them for simplicity.
>
> to underline my point once more:
> addition of bpf doesn't change at all what PERF_SAMPLE_RAW already
> delivers to user space.
> so no new ABIs anywhere.
Again, it we give an example of how to hard code the data, definitely
expect this to show up in user space. Users are going to look at this
code to learn how to use eBPF. I really want it to do it the correct
way instead of the 'easy' way. Because whatever way we have it here,
will be the way we will see it out in the wild.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Steven Rostedt @ 2015-02-10 12:27 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuzuu9T1eHyh27tdyj1+BnFta9-dzK8q7P9Y7x4iQrk+Xw@mail.gmail.com>
On Mon, 9 Feb 2015 21:51:05 -0800
Alexei Starovoitov <ast@plumgrid.com> wrote:
> On Mon, Feb 9, 2015 at 8:46 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Looks like this is entirely perf based and does not interact with
> > ftrace at all. In other words, it's perf not tracing.
> >
> > It makes more sense to go through tip than the tracing tree.
>
> well, all of earlier series were based on ftrace only,
> but I was given convincing enough arguments that
> perf_even_open+ioctl is a better interface :)
> Ok. will rebase on tip in the next version.
That's fine. If this proves useful, I'll probably add the interface
back to ftrace as well.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 3/3] lib/string_helpers.c: Change semantics of string_escape_mem
From: Andy Shevchenko @ 2015-02-10 12:32 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Andrew Morton, Trond Myklebust, J. Bruce Fields, David S. Miller,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1423525491-12613-4-git-send-email-linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
On Tue, 2015-02-10 at 00:44 +0100, Rasmus Villemoes wrote:
> The current semantics of string_escape_mem are inadequate for one of
> its current users, vsnprintf(). If that is to honour its contract, it
> must know how much space would be needed for the entire escaped
> buffer, and string_escape_mem provides no way of obtaining that (short
> of allocating a large enough buffer (~4 times input string) to let it
> play with, and that's definitely a big no-no inside vsnprintf).
>
> So change the semantics for string_escape_mem to be more
> snprintf-like: Return the size of the output that would be generated
> if the destination buffer was big enough, but of course still only
> write to the part of dst it is allowed to, and don't do
> '\0'-termination. It is then up to the caller to detect whether output
> was truncated and to append a '\0' if desired. Also, we must output
> partial escape sequences, otherwise a call such as snprintf(buf, 3,
> "%1pE", "\123") would cause printf to write a \0 to buf[2] but leaving
> buf[0] and buf[1] with whatever they previously contained.
>
> This also fixes a bug in the escaped_string() helper function, which
> used to unconditionally pass a length of "end-buf" to
> string_escape_mem(); since the latter doesn't check osz for being
> insanely large, it would happily write to dst. For example,
> kasprintf(GFP_KERNEL, "something and then %pE", ...); is an easy way
> to trigger an oops.
>
> In test-string_helpers.c, I removed the now meaningless -ENOMEM test,
> and replaced it with testing for getting the expected return value
> even if the buffer is too small. Also ensure that nothing is written
> when osz == 0.
>
> In net/sunrpc/cache.c, I think qword_add still has the same
> semantics. Someone should definitely double-check this.
Thanks for an update. My comments below.
After addressing 'em, wrt changes to patch 2/3, take my
Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
for all parts except net/sunrpc/cache.c.
>
> Signed-off-by: Rasmus Villemoes <linux-qQsb+v5E8BnlAoU/VqSP6n9LOBIZ5rWg@public.gmane.org>
> ---
> include/linux/string_helpers.h | 8 ++++----
> lib/string_helpers.c | 39 +++++++--------------------------------
> lib/test-string_helpers.c | 35 +++++++++++++++--------------------
> lib/vsprintf.c | 8 ++++++--
> net/sunrpc/cache.c | 8 +++++---
> 5 files changed, 37 insertions(+), 61 deletions(-)
>
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index 6eb567ac56bc..38a2a6f1fc76 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -47,22 +47,22 @@ static inline int string_unescape_any_inplace(char *buf)
> #define ESCAPE_ANY_NP (ESCAPE_ANY | ESCAPE_NP)
> #define ESCAPE_HEX 0x20
>
> -int string_escape_mem(const char *src, size_t isz, char **dst, size_t osz,
> +int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
> unsigned int flags, const char *esc);
>
> static inline int string_escape_mem_any_np(const char *src, size_t isz,
> - char **dst, size_t osz, const char *esc)
> + char *dst, size_t osz, const char *esc)
> {
> return string_escape_mem(src, isz, dst, osz, ESCAPE_ANY_NP, esc);
> }
>
> -static inline int string_escape_str(const char *src, char **dst, size_t sz,
> +static inline int string_escape_str(const char *src, char *dst, size_t sz,
> unsigned int flags, const char *esc)
> {
> return string_escape_mem(src, strlen(src), dst, sz, flags, esc);
> }
>
> -static inline int string_escape_str_any_np(const char *src, char **dst,
> +static inline int string_escape_str_any_np(const char *src, char *dst,
> size_t sz, const char *esc)
> {
> return string_escape_str(src, dst, sz, ESCAPE_ANY_NP, esc);
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 7e2fef1eb40e..df7fda90b333 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -278,14 +278,11 @@ static bool escape_space(unsigned char c, char **dst, char *end)
> return false;
> }
>
> - if (out + 1 >= end)
> - goto skip;
> if (out + 0 < end)
> out[0] = '\\';
> if (out + 1 < end)
> out[1] = to;
>
> -skip:
> *dst = out + 2;
> return true;
> }
> @@ -309,14 +306,11 @@ static bool escape_special(unsigned char c, char **dst, char *end)
> return false;
> }
>
> - if (out + 1 >= end)
> - goto skip;
> if (out + 0 < end)
> out[0] = '\\';
> if (out + 1 < end)
> out[1] = to;
>
> -skip:
> *dst = out + 2;
> return true;
> }
> @@ -328,14 +322,11 @@ static bool escape_null(unsigned char c, char **dst, char *end)
> if (c)
> return false;
>
> - if (out + 1 >= end)
> - goto skip;
> if (out + 0 < end)
> out[0] = '\\';
> if (out + 1 < end)
> out[1] = '0';
>
> -skip:
> *dst = out + 2;
> return true;
> }
> @@ -344,8 +335,6 @@ static bool escape_octal(unsigned char c, char **dst, char *end)
> {
> char *out = *dst;
>
> - if (out + 3 >= end)
> - goto skip;
> if (out + 0 < end)
> out[0] = '\\';
> if (out + 1 < end)
> @@ -355,7 +344,6 @@ static bool escape_octal(unsigned char c, char **dst, char *end)
> if (out + 3 < end)
> out[3] = ((c >> 0) & 0x07) + '0';
>
> -skip:
> *dst = out + 4;
> return true;
> }
> @@ -364,8 +352,6 @@ static bool escape_hex(unsigned char c, char **dst, char *end)
> {
> char *out = *dst;
>
> - if (out + 3 >= end)
> - goto skip;
> if (out + 0 < end)
> out[0] = '\\';
> if (out + 1 < end)
> @@ -375,7 +361,6 @@ static bool escape_hex(unsigned char c, char **dst, char *end)
> if (out + 3 < end)
> out[3] = hex_asc_lo(c);
>
> -skip:
> *dst = out + 4;
> return true;
> }
> @@ -429,20 +414,17 @@ skip:
> * it if needs.
> *
> * Return:
> - * The amount of the characters processed to the destination buffer, or
> - * %-ENOMEM if the size of buffer is not enough to put an escaped character is
> - * returned.
> - *
> - * Even in the case of error @dst pointer will be updated to point to the byte
> - * after the last processed character.
> + * The total size of the escaped output that would be generated for
> + * the given input and flags. To check whether the output was
> + * truncated, compare the return value to osz. There is room left in
> + * dst for a '\0' terminator if and only if ret < osz.
> */
> -int string_escape_mem(const char *src, size_t isz, char **dst, size_t osz,
> +int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
> unsigned int flags, const char *esc)
> {
> - char *p = *dst;
> + char *p = dst;
> char *end = p + osz;
> bool is_dict = esc && *esc;
> - int ret;
>
> while (isz--) {
> unsigned char c = *src++;
> @@ -482,13 +464,6 @@ int string_escape_mem(const char *src, size_t isz, char **dst, size_t osz,
> escape_passthrough(c, &p, end);
> }
>
> - if (p > end) {
> - *dst = end;
> - return -ENOMEM;
> - }
> -
> - ret = p - *dst;
> - *dst = p;
> - return ret;
> + return p - dst;
> }
> EXPORT_SYMBOL(string_escape_mem);
> diff --git a/lib/test-string_helpers.c b/lib/test-string_helpers.c
> index ab0d30e1e18f..5f759c3c2f60 100644
> --- a/lib/test-string_helpers.c
> +++ b/lib/test-string_helpers.c
> @@ -264,12 +264,12 @@ static __init void test_string_escape(const char *name,
> const struct test_string_2 *s2,
> unsigned int flags, const char *esc)
> {
> - int q_real = 512;
> - char *out_test = kmalloc(q_real, GFP_KERNEL);
> - char *out_real = kmalloc(q_real, GFP_KERNEL);
> + size_t out_size = 512;
> + char *out_test = kmalloc(out_size, GFP_KERNEL);
> + char *out_real = kmalloc(out_size, GFP_KERNEL);
> char *in = kmalloc(256, GFP_KERNEL);
> - char *buf = out_real;
> int p = 0, q_test = 0;
> + int q_real;
>
> if (!out_test || !out_real || !in)
> goto out;
> @@ -301,29 +301,26 @@ static __init void test_string_escape(const char *name,
> q_test += len;
> }
>
> - q_real = string_escape_mem(in, p, &buf, q_real, flags, esc);
> + q_real = string_escape_mem(in, p, out_real, out_size, flags, esc);
>
> test_string_check_buf(name, flags, in, p, out_real, q_real, out_test,
> q_test);
> +
> + memset(out_real, 'Z', out_size);
> + q_real = string_escape_mem(in, p, out_real, 0, flags, esc);
> + if (q_real != q_test)
> + pr_warn("Test '%s' failed: flags = %u, osz = 0, expected %d, got %d\n",
> + name, flags, q_test, q_real);
> + if (memchr_inv(out_real, 'Z', out_size))
> + pr_warn("Test '%s' failed: osz = 0 but string_escape_mem wrote to the buffer\n",
> + name);
> +
So, why couldn't we split this to separate test case? It seems I already
pointed this out.
> out:
> kfree(in);
> kfree(out_real);
> kfree(out_test);
> }
>
> -static __init void test_string_escape_nomem(void)
> -{
> - char *in = "\eb \\C\007\"\x90\r]";
> - char out[64], *buf = out;
> - int rc = -ENOMEM, ret;
> -
> - ret = string_escape_str_any_np(in, &buf, strlen(in), NULL);
> - if (ret == rc)
> - return;
> -
> - pr_err("Test 'escape nomem' failed: got %d instead of %d\n", ret, rc);
> -}
> -
> static int __init test_string_helpers_init(void)
> {
> unsigned int i;
> @@ -342,8 +339,6 @@ static int __init test_string_helpers_init(void)
> for (i = 0; i < (ESCAPE_ANY_NP | ESCAPE_HEX) + 1; i++)
> test_string_escape("escape 1", escape1, i, TEST_STRING_2_DICT_1);
>
> - test_string_escape_nomem();
> -
> return -EINVAL;
> }
> module_init(test_string_helpers_init);
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 3568e3906777..58e1193eaa4b 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1159,8 +1159,12 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
>
> len = spec.field_width < 0 ? 1 : spec.field_width;
>
> - /* Ignore the error. We print as many characters as we can */
> - string_escape_mem(addr, len, &buf, end - buf, flags, NULL);
> + /*
> + * string_escape_mem writes as many characters as it can to
'string_escape_mem() writes…'
> + * the given buffer, and returns the total size of the output
> + * had the buffer been big enough.
> + */
> + buf += string_escape_mem(addr, len, buf, buf < end ? end - buf : 0, flags, NULL);
>
> return buf;
> }
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 33fb105d4352..22c4418057f4 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -1068,12 +1068,14 @@ void qword_add(char **bpp, int *lp, char *str)
> {
> char *bp = *bpp;
> int len = *lp;
> - int ret;
> + int ret, written;
>
> if (len < 0) return;
>
> - ret = string_escape_str(str, &bp, len, ESCAPE_OCTAL, "\\ \n\t");
> - if (ret < 0 || ret == len)
> + ret = string_escape_str(str, bp, len, ESCAPE_OCTAL, "\\ \n\t");
> + written = min(ret, len);
> + bp += written;
> + if (ret >= len)
> len = -1;
> else {
> len -= ret;
--
Andy Shevchenko <andriy.shevchenko-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [patch added to the 3.12 stable tree] lib/checksum.c: fix carry in csum_tcpudp_nofold
From: Jiri Slaby @ 2015-02-10 10:17 UTC (permalink / raw)
To: stable
Cc: karl beldan, Karl Beldan, Al Viro, Eric Dumazet, Arnd Bergmann,
Mike Frysinger, netdev, linux-kernel, Eric Dumazet,
David S. Miller, Jiri Slaby
In-Reply-To: <1423563470-21303-1-git-send-email-jslaby@suse.cz>
From: karl beldan <karl.beldan@gmail.com>
This patch has been added to the 3.12 stable tree. If you have any
objections, please let us know.
===============
commit 150ae0e94634714b23919f0c333fee28a5b199d5 upstream.
The carry from the 64->32bits folding was dropped, e.g with:
saddr=0xFFFFFFFF daddr=0xFF0000FF len=0xFFFF proto=0 sum=1,
csum_tcpudp_nofold returned 0 instead of 1.
Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
lib/checksum.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/checksum.c b/lib/checksum.c
index 129775eb6de6..fcf38943132c 100644
--- a/lib/checksum.c
+++ b/lib/checksum.c
@@ -47,6 +47,15 @@ static inline unsigned short from32to16(unsigned int x)
return x;
}
+static inline u32 from64to32(u64 x)
+{
+ /* add up 32-bit and 32-bit for 32+c bit */
+ x = (x & 0xffffffff) + (x >> 32);
+ /* add up carry.. */
+ x = (x & 0xffffffff) + (x >> 32);
+ return (u32)x;
+}
+
static unsigned int do_csum(const unsigned char *buff, int len)
{
int odd;
@@ -195,8 +204,7 @@ __wsum csum_tcpudp_nofold(__be32 saddr, __be32 daddr,
#else
s += (proto + len) << 8;
#endif
- s += (s >> 32);
- return (__force __wsum)s;
+ return (__force __wsum)from64to32(s);
}
EXPORT_SYMBOL(csum_tcpudp_nofold);
#endif
--
2.2.2
^ permalink raw reply related
* Re: [PATCH] prevent the read ahead of /proc/slabinfo in ss take 2
From: Sergei Shtylyov @ 2015-02-10 12:37 UTC (permalink / raw)
To: Bryton Lee, stephen, netdev, davem; +Cc: eric.dumazet
In-Reply-To: <1423536360-5298-1-git-send-email-brytonlee01@gmail.com>
Hello.
On 2/10/2015 5:46 AM, Bryton Lee wrote:
> ss reads ahead of /proc/slabinfo whatever slabstat will be used or not in future.
> this will cause huge system delay when the kernel hires SLAB allocator(SLUB allocator is ok). when program reads
> /proc/slabinfo, it will call s_show() in SLAB allocator level, and s_show() calls spin_lock_irq(&l3->list_lock)
> then iterate on whole three lists. if one slab has about 900 million objects (for example dentry),
> it will cause more than 1000ms delay.
> so this patch prevents the read ahead of /proc/slabinfo, ss runs with most parameters not using slabstat at all.
> and this patch also change slabstat and slabstat_valid to static.
> Signed-off-by: Bryton Lee <brytonlee01@gmail.com>
> ---
> misc/ss.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
> diff --git a/misc/ss.c b/misc/ss.c
> index 7fc0a99..5fa6259 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -616,7 +616,8 @@ struct slabstat
> int skbs;
> };
>
> -struct slabstat slabstat;
> +static struct slabstat slabstat;
> +static int slabstat_valid = 0;
No need to initialize to 0.
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] prevent the read ahead of /proc/slabinfo in ss - take 3
From: Sergei Shtylyov @ 2015-02-10 12:51 UTC (permalink / raw)
To: Bryton Lee, stephen, netdev, davem; +Cc: eric.dumazet
In-Reply-To: <1423547533-6727-1-git-send-email-brytonlee01@gmail.com>
On 2/10/2015 8:52 AM, Bryton Lee wrote:
> prevent the read ahead of /proc/slabinfo in ss.
> Signed-off-by: Bryton Lee <brytonlee01@gmail.com>
> ---
> misc/ss.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
> diff --git a/misc/ss.c b/misc/ss.c
> index 7fc0a99..74721b5 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
[...]
> @@ -655,6 +656,8 @@ static int get_slabstat(struct slabstat *s)
> break;
> }
>
> + slabstat_valid = 1;
This is indented incorrectly. Use one TAB please.
> +
> fclose(fp);
> return 0;
> }
> @@ -2230,6 +2233,9 @@ static int tcp_show(struct filter *f, int socktype)
> * it is able to give us some memory for snapshot.
> */
> if (1) {
> + if (!slabstat_valid)
> + get_slabstat(&slabstat);
> +
> int guess = slabstat.socks+slabstat.tcp_syns;
> if (f->states&(1<<SS_TIME_WAIT))
> guess += slabstat.tcp_tws;
> @@ -3196,6 +3202,9 @@ static int print_summary(void)
> if (get_snmp_int("Tcp:", "CurrEstab", &sn.tcp_estab) < 0)
> perror("ss: get_snmpstat");
>
> + if (!slabstat_valid)
> + get_slabstat(&slabstat);
> +
Can't you perform this check inside get_slabstat()?
[...]
WBR, Sergei
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Eric Dumazet @ 2015-02-10 12:54 UTC (permalink / raw)
To: Michal Kazior
Cc: Neal Cardwell, linux-wireless, Network Development, Eyal Perry
In-Reply-To: <CA+BoTQ=u_xPuqTVOVaFTQNRrJ+UTXe89SY+=+7Y1LpxxrkRDfg@mail.gmail.com>
On Tue, 2015-02-10 at 11:33 +0100, Michal Kazior wrote:
> + if (msdu->sk) {
> + ewma_add(&ar->tx_delay_us,
> + ktime_to_ns(ktime_sub(ktime_get(), skb_cb->stamp)) /
> + NSEC_PER_USEC);
> +
> + ACCESS_ONCE(msdu->sk->sk_tx_completion_delay_cushion) =
> + (ewma_read(&ar->tx_delay_us) *
> + msdu->sk->sk_pacing_rate) >> 20;
> + }
> +
Hi Michal
This is almost it ;)
As I said you must do this using u64 arithmetics, we still support 32bit
kernels.
Also, >> 20 instead of / 1000000 introduces a 5% error, I would use a
plain divide, as the compiler will use a reciprocal divide (ie : a
multiply)
We use >> 10 instead of /1000 because a 2.4 % error is probably okay.
ewma_add(&ar->tx_delay_us,
ktime_to_ns(ktime_sub(ktime_get(),
skb_cb->stamp)) /
NSEC_PER_USEC);
u64 val = (u64)ewma_read(&ar->tx_delay_us) *
msdu->sk->sk_pacing_rate;
do_div(val, USEC_PER_SEC);
ACCESS_ONCE(msdu->sk->sk_tx_completion_delay_cushion) =
(u32)val;
(WRITE_ONCE() would be better for new kernels, but ACCESS_ONCE() is ok
since we probably want to backport to stable kernels)
Thanks
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: Rename sock_recv_ts_and_drops() to sock_cmsg_recv()
From: Eyal Birger @ 2015-02-10 12:57 UTC (permalink / raw)
To: David Miller; +Cc: edumazet, netdev
In-Reply-To: <20150209.185535.1300672977479433508.davem@davemloft.net>
Hi,
> On Feb 10, 2015, at 4:55 AM, David Miller <davem@davemloft.net> wrote:
>
> From: Eyal Birger <eyal.birger@gmail.com>
> Date: Mon, 9 Feb 2015 20:15:21 +0200
>
>> -void __sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
>> +void __sock_cmsg_recv(struct msghdr *msg, struct sock *sk,
>> struct sk_buff *skb);
>
> If you change the column where the openning parenthesis of a function
> declaration appears, you have to reindent the subsequent lines,
> if any, so that the arguments still begin at the first column after
> that openning parenthesis.
Sorry about that, thanks! Will fix.
> Also, this new feature is too late for this merge window, so there is
> no rush for resubmitting this.
Thanks. Should I wait for the next merge window or may I resubmit at this point?
Regards,
Eyal.
^ permalink raw reply
* Re: [PATCH v3 3/3] lib/string_helpers.c: Change semantics of string_escape_mem
From: Rasmus Villemoes @ 2015-02-10 13:02 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andrew Morton, Trond Myklebust, J. Bruce Fields, David S. Miller,
linux-kernel, linux-nfs, netdev
In-Reply-To: <1423571552.31903.468.camel@linux.intel.com>
On Tue, Feb 10 2015, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2015-02-10 at 00:44 +0100, Rasmus Villemoes wrote:
>> The current semantics of string_escape_mem are inadequate for one of
>> its current users, vsnprintf(). If that is to honour its contract, it
>> must know how much space would be needed for the entire escaped
>> buffer, and string_escape_mem provides no way of obtaining that (short
>> of allocating a large enough buffer (~4 times input string) to let it
>> play with, and that's definitely a big no-no inside vsnprintf).
>>
>> So change the semantics for string_escape_mem to be more
>> snprintf-like: Return the size of the output that would be generated
>> if the destination buffer was big enough, but of course still only
>> write to the part of dst it is allowed to, and don't do
>> '\0'-termination. It is then up to the caller to detect whether output
>> was truncated and to append a '\0' if desired. Also, we must output
>> partial escape sequences, otherwise a call such as snprintf(buf, 3,
>> "%1pE", "\123") would cause printf to write a \0 to buf[2] but leaving
>> buf[0] and buf[1] with whatever they previously contained.
>>
>> This also fixes a bug in the escaped_string() helper function, which
>> used to unconditionally pass a length of "end-buf" to
>> string_escape_mem(); since the latter doesn't check osz for being
>> insanely large, it would happily write to dst. For example,
>> kasprintf(GFP_KERNEL, "something and then %pE", ...); is an easy way
>> to trigger an oops.
>>
>> In test-string_helpers.c, I removed the now meaningless -ENOMEM test,
>> and replaced it with testing for getting the expected return value
>> even if the buffer is too small. Also ensure that nothing is written
>> when osz == 0.
>>
>> In net/sunrpc/cache.c, I think qword_add still has the same
>> semantics. Someone should definitely double-check this.
>
> Thanks for an update. My comments below.
> After addressing 'em, wrt changes to patch 2/3, take my
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> for all parts except net/sunrpc/cache.c.
>
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>> index ab0d30e1e18f..5f759c3c2f60 100644
>> --- a/lib/test-string_helpers.c
>> +++ b/lib/test-string_helpers.c
>> @@ -264,12 +264,12 @@ static __init void test_string_escape(const char *name,
>> const struct test_string_2 *s2,
>> unsigned int flags, const char *esc)
>> {
>> - int q_real = 512;
>> - char *out_test = kmalloc(q_real, GFP_KERNEL);
>> - char *out_real = kmalloc(q_real, GFP_KERNEL);
>> + size_t out_size = 512;
>> + char *out_test = kmalloc(out_size, GFP_KERNEL);
>> + char *out_real = kmalloc(out_size, GFP_KERNEL);
>> char *in = kmalloc(256, GFP_KERNEL);
>> - char *buf = out_real;
>> int p = 0, q_test = 0;
>> + int q_real;
>>
>> if (!out_test || !out_real || !in)
>> goto out;
>> @@ -301,29 +301,26 @@ static __init void test_string_escape(const char *name,
>> q_test += len;
>> }
>>
>> - q_real = string_escape_mem(in, p, &buf, q_real, flags, esc);
>> + q_real = string_escape_mem(in, p, out_real, out_size, flags, esc);
>>
>> test_string_check_buf(name, flags, in, p, out_real, q_real, out_test,
>> q_test);
>> +
>> + memset(out_real, 'Z', out_size);
>> + q_real = string_escape_mem(in, p, out_real, 0, flags, esc);
>> + if (q_real != q_test)
>> + pr_warn("Test '%s' failed: flags = %u, osz = 0, expected %d, got %d\n",
>> + name, flags, q_test, q_real);
>> + if (memchr_inv(out_real, 'Z', out_size))
>> + pr_warn("Test '%s' failed: osz = 0 but string_escape_mem wrote to the buffer\n",
>> + name);
>> +
>
> So, why couldn't we split this to separate test case? It seems I already
> pointed this out.
>
This actually provides better coverage since we do this for all the
"positive" test cases, instead of just the single ad hoc case done previously. Of
course the added lines could be factored into a separate helper, but
there's quite a lot of state to pass, so I thought this would actually
be simpler - note how the two string_escape_mem calls are easily seen to
be identical except for the outsize argument.
It may already be too late for the merge window, but I didn't want to
spend too much time on these mostly cosmetic details (that also goes for
the 3- versus 2-line issue).
Rasmus
^ 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