* Re: [PATCH 2/8] net: ethernet: stmmac: update to support all PHY config for stm32mp157c.
From: Christophe ROULLIER @ 2019-02-14 15:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh@kernel.org, davem@davemloft.net, joabreu@synopsys.com,
mark.rutland@arm.com, mcoquelin.stm32@gmail.com, Alexandre TORGUE,
Peppe CAVALLARO, linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190214151540.GG708@lunn.ch>
On 2/14/19 4:15 PM, Andrew Lunn wrote:
>> Sorry, I've misunderstood your question ;-)
>>
>> And you spoke about :
>>
>> case PHY_INTERFACE_MODE_RGMII:
>> case PHY_INTERFACE_MODE_RGMII_ID:
>> case PHY_INTERFACE_MODE_RGMII_RXID:
>> case PHY_INTERFACE_MODE_RGMII_TXID:
>>
>> So in my setup I've only RGMII interface, so I've never tested 3 others
>> interfaces (_ID, _RXID, _TXID).
>>
>> So do I need to add cases in my driver ?
>
> Yes. They all indicate the MAC should be using RGMII.
>
> This appears to be an old issue, but now would be a good time to fix
> it.
>
> Andrew
>
Ok Andrew, I will update with these
Thanks
Christophe
^ permalink raw reply
* Re: [rdma-rc PATCH 1/2] cxgb4: export sge_host_page_size to ulds
From: Leon Romanovsky @ 2019-02-14 15:51 UTC (permalink / raw)
To: Raju Rangoju; +Cc: jgg, davem, linux-rdma, netdev, swise
In-Reply-To: <20190214121054.11693-2-rajur@chelsio.com>
[-- Attachment #1: Type: text/plain, Size: 334 bytes --]
On Thu, Feb 14, 2019 at 05:40:53PM +0530, Raju Rangoju wrote:
> Export the sge_host_page_size field to ULDs via
> cxgb4_lld_info, so that iw_cxgb4 can make use of
> this in calculating the correct qp/cq mask.
>
> Fixes: 2391b0030e ("cxgb4: Remove SGE_HOST_PAGE_SIZE dependency on page
> size")
Please don't break Fixes line.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* RE: [PATCH net-next 3/3] enetc: Add ENETC PF level external MDIO support
From: Claudiu Manoil @ 2019-02-14 15:48 UTC (permalink / raw)
To: Andrew Lunn
Cc: Shawn Guo, Leo Li, David S . Miller, devicetree@vger.kernel.org,
Alexandru Marginean, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190213181323.GA708@lunn.ch>
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Wednesday, February 13, 2019 8:13 PM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; David S .
>Miller <davem@davemloft.net>; devicetree@vger.kernel.org; Alexandru
>Marginean <alexandru.marginean@nxp.com>; linux-kernel@vger.kernel.org;
>linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org
>Subject: Re: [PATCH net-next 3/3] enetc: Add ENETC PF level external MDIO
>support
>
[...]
>> +static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
>> + u16 value)
>> +{
>> + struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
>> + u32 mdio_ctl, mdio_cfg;
>> + u16 dev_addr;
>> + int ret;
>> +
>> + mdio_cfg = MDIO_CFG_CLKDIV(ENETC_MDC_DIV) | MDIO_CFG_NEG;
>
>What does MDIO_CFG_NEG mean?
>
I'll add a comment in the code to clarify this define.
It means the MDIO is driven by master on MDC negative edge, which is how
the external mdio busses work on this hardware. For internal MDIO CFG_NEG is 0.
[...]
>
>Maybe MDIO_CFG_ENC could be called MDIO_CFG_C45? Assuming that is what
>it actually means?
>
You're right, in the hardware manual this bit is actually called "ENC45", so
I'll change the define name to that. Should be more clear like this.
[...]
>
>It is a good idea to have an mdio node which contains the list of
>PHYs. You can get into odd situations if you don't do that.
>
Hopefully this doesn't complicate things since these are not platform devices.
It's not as easy as adding new platform device driver for mdio...
Ok for the rest of the comments too.
Thanks for the review.
Claudiu
^ permalink raw reply
* Re: [rdma-rc PATCH 2/2] iw_cxgb4: cq/qp mask depends on bar2 pages in a host page
From: Jason Gunthorpe @ 2019-02-14 15:41 UTC (permalink / raw)
To: Raju Rangoju
Cc: davem@davemloft.net, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, swise@opengridcomputing.com
In-Reply-To: <20190214121054.11693-3-rajur@chelsio.com>
On Thu, Feb 14, 2019 at 05:40:54PM +0530, Raju Rangoju wrote:
> Adjust the cq/qp mask based on no.of bar2 pages in a host page.
>
> For user-mode rdma, the granularity of the BAR2 memory mapped
> to a user rdma process during queue allocation must be based
> on the host page size. The lld attributes udb_density and
> ucq_density are used to figure out how many sge contexts are
> in a bar2 page. So the rdev->qpmask and rdev->cqmask in
> iw_cxgb4 need to now be adjusted based on how many sge bar2
> pages are in a host page.
Why is this rc? Do certain arches fail to work or something?
Jason
^ permalink raw reply
* Re: [PATCH] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: Alexander Duyck @ 2019-02-14 15:37 UTC (permalink / raw)
To: Jann Horn
Cc: linux-mm, Andrew Morton, LKML, Michal Hocko, Vlastimil Babka,
Pavel Tatashin, Oscar Salvador, Mel Gorman, Aaron Lu, Netdev,
Alexander Duyck
In-Reply-To: <CAG48ez0v7QwtCKDs5vgRJht8yfZR5nudEpkMOLaDX-=47WeFqA@mail.gmail.com>
On Thu, Feb 14, 2019 at 7:13 AM Jann Horn <jannh@google.com> wrote:
>
> On Wed, Feb 13, 2019 at 11:42 PM Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> > On Wed, Feb 13, 2019 at 12:42 PM Jann Horn <jannh@google.com> wrote:
> > > The basic idea behind ->pagecnt_bias is: If we pre-allocate the maximum
> > > number of references that we might need to create in the fastpath later,
> > > the bump-allocation fastpath only has to modify the non-atomic bias value
> > > that tracks the number of extra references we hold instead of the atomic
> > > refcount. The maximum number of allocations we can serve (under the
> > > assumption that no allocation is made with size 0) is nc->size, so that's
> > > the bias used.
> > >
> > > However, even when all memory in the allocation has been given away, a
> > > reference to the page is still held; and in the `offset < 0` slowpath, the
> > > page may be reused if everyone else has dropped their references.
> > > This means that the necessary number of references is actually
> > > `nc->size+1`.
> > >
> > > Luckily, from a quick grep, it looks like the only path that can call
> > > page_frag_alloc(fragsz=1) is TAP with the IFF_NAPI_FRAGS flag, which
> > > requires CAP_NET_ADMIN in the init namespace and is only intended to be
> > > used for kernel testing and fuzzing.
> >
> > Actually that has me somewhat concerned. I wouldn't be surprised if
> > most drivers expect the netdev_alloc_frags call to at least output an
> > SKB_DATA_ALIGN sized value.
> >
> > We probably should update __netdev_alloc_frag and __napi_alloc_frag so
> > that they will pass fragsz through SKB_DATA_ALIGN.
>
> Do you want to do a separate patch for that? I'd like to not mix
> logically separate changes in a single patch, and I also don't have a
> good understanding of the alignment concerns here.
You could just include it as a separate patch with your work.
Otherwise I will get to it when I have time.
The point is the issue you pointed out will actually cause other
issues if the behavior is maintained since you shouldn't be getting
unaligned blocks out of the frags API anyway.
> > > To test for this issue, put a `WARN_ON(page_ref_count(page) == 0)` in the
> > > `offset < 0` path, below the virt_to_page() call, and then repeatedly call
> > > writev() on a TAP device with IFF_TAP|IFF_NO_PI|IFF_NAPI_FRAGS|IFF_NAPI,
> > > with a vector consisting of 15 elements containing 1 byte each.
> > >
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jann Horn <jannh@google.com>
> > > ---
> > > mm/page_alloc.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index 35fdde041f5c..46285d28e43b 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -4675,11 +4675,11 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> > > /* Even if we own the page, we do not use atomic_set().
> > > * This would break get_page_unless_zero() users.
> > > */
> > > - page_ref_add(page, size - 1);
> > > + page_ref_add(page, size);
> > >
> > > /* reset page count bias and offset to start of new frag */
> > > nc->pfmemalloc = page_is_pfmemalloc(page);
> > > - nc->pagecnt_bias = size;
> > > + nc->pagecnt_bias = size + 1;
> > > nc->offset = size;
> > > }
> > >
> > > @@ -4695,10 +4695,10 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> > > size = nc->size;
> > > #endif
> > > /* OK, page count is 0, we can safely set it */
> > > - set_page_count(page, size);
> > > + set_page_count(page, size + 1);
> > >
> > > /* reset page count bias and offset to start of new frag */
> > > - nc->pagecnt_bias = size;
> > > + nc->pagecnt_bias = size + 1;
> > > offset = size - fragsz;
> > > }
> >
> > If we already have to add a constant it might be better to just use
> > PAGE_FRAG_CACHE_MAX_SIZE + 1 in all these spots where you are having
> > to use "size + 1" instead of "size". That way we can avoid having to
> > add a constant to a register value and then program that value.
> > instead we can just assign the constant value right from the start.
>
> I doubt that these few instructions make a difference, but sure, I can
> send a v2 with that changed.
You would be surprised. They all end up adding up over time.
^ permalink raw reply
* RE: [PATCH net-next 2/3] arm64: dts: fsl: ls1028a-rdb: Add ENETC external eth ports for the LS1028A RDB board
From: Claudiu Manoil @ 2019-02-14 15:33 UTC (permalink / raw)
To: Andrew Lunn
Cc: Shawn Guo, Leo Li, David S . Miller, devicetree@vger.kernel.org,
Alexandru Marginean, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190213181554.GB708@lunn.ch>
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Wednesday, February 13, 2019 8:16 PM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; David S .
>Miller <davem@davemloft.net>; devicetree@vger.kernel.org; Alexandru
>Marginean <alexandru.marginean@nxp.com>; linux-kernel@vger.kernel.org;
>linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org
>Subject: Re: [PATCH net-next 2/3] arm64: dts: fsl: ls1028a-rdb: Add ENETC
>external eth ports for the LS1028A RDB board
>
>On Wed, Feb 13, 2019 at 01:02:22PM +0200, Claudiu Manoil wrote:
>> The LS1028A RDB board features an Atheros PHY connected over SGMII to
>> the ENETC PF0 (or Port0). ENETC Port1 (PF1) has no external
>> connection on this board, so it can be disabled for now.
>>
>> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
>> ---
>> arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 15
>> +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
>> b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
>> index fdeb417..c8487893 100644
>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
>> @@ -71,3 +71,18 @@
>> &duart1 {
>> status = "okay";
>> };
>> +
>> +&enetc_port0 {
>> + phy-handle = <&sgmii_phy0>;
>> + phy-connection-type = "sgmii";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + sgmii_phy0: ethernet-phy@2 {
>> + reg = <0x2>;
>> + };
>> +};
>> +
>
>Hi Claudiu
>
>It is better to use:
>
>&enetc_port0 {
> phy-handle = <&sgmii_phy0>;
> phy-connection-type = "sgmii";
> #address-cells = <1>;
> #size-cells = <0>;
>
> mdio {
> sgmii_phy0: ethernet-phy@2 {
> reg = <0x2>;
> };
> };
>};
Hi Andrew,
The extra node for mdio seems to complicate things somewhat.
Just adding this node seems not enough. How to find out easily if a child
of a enetc port node is a mdio node? I was thinking to use device_type
for that, like this:
&enetc_port0 {
phy-handle = <&sgmii_phy0>;
phy-connection-type = "sgmii";
mdio {
#address-cells = <1>;
#size-cells = <0>;
device_type = "mdio";
sgmii_phy0: ethernet-phy@2 {
reg = <0x2>;
};
};
};
Would you agree with this?
Thanks,
Claudiu
^ permalink raw reply
* Re: [net-next, PATCH] net: stmmac: use correct define to get rx timestamp on GMAC4
From: Jose Abreu @ 2019-02-14 15:30 UTC (permalink / raw)
To: Alexandre Torgue, Jose Abreu, Giuseppe Cavallaro, davem
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <a62c35de-7afa-14ec-5ca6-05bd61b6d09d@st.com>
On 2/14/2019 3:00 PM, Alexandre Torgue wrote:
> Hi Jose
>
> On 2/14/19 3:18 PM, Jose Abreu wrote:
>> Hi Alexandre,
>>
>> On 2/14/2019 2:12 PM, Alexandre Torgue wrote:
>>> In dwmac4_wrback_get_rx_timestamp_status we looking for a RX
>>> timestamp.
>>> For that receive descriptors are handled and so we should use
>>> defines
>>> related to receive descriptors. It'll no change the functional
>>> behavior
>>> as RDES3_RDES1_VALID=TDES3_RS1V=BIT(26) but it makes code
>>> easier to read.
>>>
>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>>
>>> diff --git
>>> a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>> index 20299f6..9f062b3 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>> @@ -268,7 +268,7 @@ static int
>>> dwmac4_wrback_get_rx_timestamp_status(void *desc, void
>>> *next_desc,
>>> int ret = -EINVAL;
>>> /* Get the status from normal w/b descriptor */
>>> - if (likely(p->des3 & TDES3_RS1V)) {
>>> + if (likely(p->des3 & RDES3_RDES1_VALID)) {
>>
>> Shouldn't this also use le32_to_cpu() like bellow ?
>
> I agree. I focused on cosmetic but yes you are right, we have to
> take car about endianness as this IP is used by different
> processors (using different endianness). I gonna send a v2.
> I think dwmac4_rx_check_timestamp have the same kind of issue.
> Another patch should be sent for it. no ?
Yeah. Maybe you can send all of that in this v2 patch also ?
Thanks,
Jose Miguel Abreu
>
> regards
> Alex
>
>
>
>>
>> Thanks,
>> Jose Miguel Abreu
>>
>>> if (likely(le32_to_cpu(p->des1) &
>>> RDES1_TIMESTAMP_AVAILABLE)) {
>>> int i = 0;
>>>
^ permalink raw reply
* Re: KASAN: use-after-free Read in sctp_outq_tail
From: Marcelo Ricardo Leitner @ 2019-02-14 15:27 UTC (permalink / raw)
To: Xin Long
Cc: syzbot, davem, LKML, linux-sctp, network dev, Neil Horman,
syzkaller-bugs, Vlad Yasevich
In-Reply-To: <CADvbK_ctRtcV3Fz6CwAoiTtVH3h5u7gj=6BsiZSR3Qdq+2rDMA@mail.gmail.com>
On Thu, Feb 14, 2019 at 10:52:00PM +0800, Xin Long wrote:
> On Thu, Feb 14, 2019 at 10:39 PM Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> >
> > On Thu, Feb 14, 2019 at 12:17:45PM -0200, Marcelo Ricardo Leitner wrote:
> > > On Thu, Feb 14, 2019 at 10:03:30PM +0800, Xin Long wrote:
> > > > On Wed, Feb 13, 2019 at 9:52 PM Marcelo Ricardo Leitner
> > > > <marcelo.leitner@gmail.com> wrote:
> > > > >
> > > > > On Wed, Feb 13, 2019 at 12:35:56PM +0800, Xin Long wrote:
> > > > > > On Wed, Feb 13, 2019 at 4:00 AM syzbot
> > > > > > <syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com> wrote:
> > > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > > syzbot found the following crash on:
> > > > > > >
> > > > > > > HEAD commit: d4104460aec1 Add linux-next specific files for 20190211
> > > > > > > git tree: linux-next
> > > > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=14140124c00000
> > > > > > > kernel config: https://syzkaller.appspot.com/x/.config?x=c8a112d3b0d6719b
> > > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=7823fa3f3e2d69341ea8
> > > > > > > compiler: gcc (GCC) 9.0.0 20181231 (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+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com
> > > > > > >
> > > > > > > ==================================================================
> > > > > > > BUG: KASAN: use-after-free in list_add_tail include/linux/list.h:93 [inline]
> > > > > > > BUG: KASAN: use-after-free in sctp_outq_tail_data net/sctp/outqueue.c:105
> > > > > > > [inline]
> > > > > > > BUG: KASAN: use-after-free in sctp_outq_tail+0x816/0x930
> > > > > > > net/sctp/outqueue.c:313
> > > > > > > Read of size 8 at addr ffff88807b19a7b8 by task syz-executor.0/30745
> > > > > > I think https://patchwork.ozlabs.org/patch/1040500/ will fix this.
> > > > >
> > > > > I don't think so. Seems it will switch from use-after-free to NULL deref
> > > > > instead with that patch.
> > > > It will allocate ext for the stream if its ext is NULL in
> > > > sctp_sendmsg_to_asoc(), the new data will work fine. As for
> > >
> > > Ehh, right!
> > >
> > > > the old data on the surplus streams, it has been dropped in
> > > > sctp_stream_outq_migrate().
> > >
> > > Yep.
> > >
> > > >
> > > > >
> > > > > > > CPU: 1 PID: 30745 Comm: syz-executor.0 Not tainted 5.0.0-rc5-next-20190211
> > > > > > > #32
> > > > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > > > > Google 01/01/2011
> > > > > > > Call Trace:
> > > > > > > __dump_stack lib/dump_stack.c:77 [inline]
> > > > > > > dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> > > > > > > print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
> > > > > > > kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> > > > > > > __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
> > > > > > > list_add_tail include/linux/list.h:93 [inline]
> > > > > > > sctp_outq_tail_data net/sctp/outqueue.c:105 [inline]
> > > > > > > sctp_outq_tail+0x816/0x930 net/sctp/outqueue.c:313
> > > > > > > sctp_cmd_send_msg net/sctp/sm_sideeffect.c:1109 [inline]
> > > > > > > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1784 [inline]
> > > > > > > sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> > > > > > > sctp_do_sm+0x68e/0x5380 net/sctp/sm_sideeffect.c:1191
> > > > > > > sctp_primitive_SEND+0xa0/0xd0 net/sctp/primitive.c:178
> > > > > > > sctp_sendmsg_to_asoc+0xa63/0x17b0 net/sctp/socket.c:1955
> > > > >
> > > > > sctp_sendmsg_to_asoc()
> > > > > ...
> > > > > if (sinfo->sinfo_stream >= asoc->stream.outcnt) {
> > > > > err = -EINVAL;
> > > > > goto err;
> > > > > }
> > > > >
> > > > > if (unlikely(!SCTP_SO(&asoc->stream, sinfo->sinfo_stream)->ext)) {
> > > > > ...
> > > > >
> > > > > It should have aborted even if an old ->ext was still there because
> > > > > outcnt is correctly updated. So somehow outcnt was wrong here.
> > > > >
> > > > > sctp_stream_init()
> > > > > ...
> > > > > /* Filter out chunks queued on streams that won't exist anymore */
> > > > > sched->unsched_all(stream);
> > > > > sctp_stream_outq_migrate(stream, NULL, outcnt); <--- [A]
> > > > > sched->sched_all(stream);
> > > > >
> > > > > ret = sctp_stream_alloc_out(stream, outcnt, gfp); <--- [B]
> > > > > if (ret)
> > > > > goto out;
> > > > >
> > > > > stream->outcnt = outcnt; <--- [C]
> > > > > ...
> > > > >
> > > > > We have a problem here because [A] is freeing ->ext, but [B] can fail (ENOMEM),
> > > > > which would lead it to not update outcnt in [C] even after the
> > > > > changes already performed in [A].
> > > > >
> > > > > It should handle the freeing of ->ext in sctp_stream_alloc_out()
> > > > > instead, or allocate the flexarray earlier (so it can bail out before
> > > > > freeing stuff).
> > > > Agree that it's better to do:
> > > > sched->unsched_all(stream);
> > > > sctp_stream_outq_migrate(stream, NULL, outcnt);
> > > > sched->sched_all(stream);
> > > > after the flexarray allocation.
> > > >
> > > > Just sctp_stream_alloc_out() can not be used here anymore, as
> > > > stream->out will be set in it.
> > >
> > > What about carving out a sctp_stream_init_out() ? Like
> > >
> > > struct flex_array *new_out;
> > >
> > > /* just allocate it, nothing more */
> > > new_out = sctp_stream_alloc_out(outcnt, gfp);
> > > if (!new_out)
> > > goto out;
> > >
> > > /* Filter out chunks queued on streams that won't exist anymore */
> > > sched->unsched_all(stream);
> > > sctp_stream_outq_migrate(stream, NULL, outcnt);
> > > sched->sched_all(stream);
> > >
> > > /* initialization stuff, and it can't fail anymore */
> > > sctp_stream_init_out(stream, outcnt, new_out);
> > >
> > > This may hurt a bit more with the genradix migration, but then we
> > > don't end up dropping data for nothing.
> >
> > Reviewing calls to this function, if this allocation fails it will
> > either cause the asoc to be terminated or sendmsg error. So data loss
> > is not really an issue here.
> >
> sctp_stream_init+0xbc/0x410 net/sctp/stream.c:139
> sctp_process_init+0x21c3/0x2b20 net/sctp/sm_make_chunk.c:2466
> sctp_cmd_process_init net/sctp/sm_sideeffect.c:682 [inline]
> sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1410 [inline]
>
> on this path, seems that it won't terminate the asoc nor report a sending error.
Seems that's only triggered from sctp_sf_do_5_1C_ack() and quite
interesting. If we can't perform the reduction of the number of output
streams, we will be violating the handshake. Considering the error
will cause it to stop processing the commands from the state machine,
it won't output cookie echo pkt and this asoc will be left in a
somewhat funny state. I think the fact that it won't stop the T1
then, allows it to get corrected later on.
But that err_chunk, if any, gets leaked.
^ permalink raw reply
* Re: [PATCH net] dsa: mv88e6xxx: Ensure all pending interrupts are handled prior to exit
From: David Miller @ 2019-02-14 15:27 UTC (permalink / raw)
To: andrew; +Cc: dave.anglin, linux, vivien.didelot, f.fainelli, netdev
In-Reply-To: <20190214045019.GC20024@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Thu, 14 Feb 2019 05:50:19 +0100
>> Should I queue just this one for -stable? I didn't queue up Heiner's change for
>> -stable because it fixes a 5.0-rcX regression.
>
> Yes please.
Done.
^ permalink raw reply
* Re: TC stats / hw offload question
From: Andy Gospodarek @ 2019-02-14 15:17 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Edward Cree, netdev, Jiri Pirko, Cong Wang, Or Gerlitz,
PJ Waskiewicz, Anjali Singhai Jain, Jakub Kicinski
In-Reply-To: <702dd5b7-c6ed-b669-8270-d44f5ff4fb30@mojatatu.com>
On Thu, Feb 14, 2019 at 07:39:28AM -0500, Jamal Hadi Salim wrote:
>
> On 2019-02-11 6:44 a.m., Edward Cree wrote:
>
> > But since the
> > other vendors don't seem to do that, I wondered if there was a
> > reason, or if perhaps the counter resources (and PCI bw to read
> > them) could be saved if all those separate counters aren't really
> > needed.
>
> Probably nobody has paid attention or asked as you did.
That's not totally true, but I'm glad to hear that others are
considering it.
> Will let the h/w folks speak for themselves. My understanding
> based on experience is counters are cheap. Most modern NICs
> and ASICs have a gazillion of them at their disposal.
Counters can be cheap to implement (though that does not always mean
that everyone chooses to add many of them to hardware), but the real
cost to them, as Edward points out, is the cost of accessing them an
keeping them up to date. If we are concerned about keeping track of
flow counters on thousands (or more) flows the cost on the PCI bus can
be quite high.
We have been looking at a few options to deal with tracking this massive
number of counts, but are open to hearing what others feel they would
like to see happen to this. Though stats sometimes seem boring to some
developers, they are critical and we have been considering whether or
not we need to think about different driver or global infra to handle
it.
^ permalink raw reply
* Re: [PATCH 2/8] net: ethernet: stmmac: update to support all PHY config for stm32mp157c.
From: Andrew Lunn @ 2019-02-14 15:15 UTC (permalink / raw)
To: Christophe ROULLIER
Cc: robh@kernel.org, davem@davemloft.net, joabreu@synopsys.com,
mark.rutland@arm.com, mcoquelin.stm32@gmail.com, Alexandre TORGUE,
Peppe CAVALLARO, linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <02cd7773-899f-b28a-ff61-849b17cbe82d@st.com>
> Sorry, I've misunderstood your question ;-)
>
> And you spoke about :
>
> case PHY_INTERFACE_MODE_RGMII:
> case PHY_INTERFACE_MODE_RGMII_ID:
> case PHY_INTERFACE_MODE_RGMII_RXID:
> case PHY_INTERFACE_MODE_RGMII_TXID:
>
> So in my setup I've only RGMII interface, so I've never tested 3 others
> interfaces (_ID, _RXID, _TXID).
>
> So do I need to add cases in my driver ?
Yes. They all indicate the MAC should be using RGMII.
This appears to be an old issue, but now would be a good time to fix
it.
Andrew
^ permalink raw reply
* Re: [PATCH] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: Jann Horn @ 2019-02-14 15:13 UTC (permalink / raw)
To: Alexander Duyck
Cc: linux-mm, Andrew Morton, LKML, Michal Hocko, Vlastimil Babka,
Pavel Tatashin, Oscar Salvador, Mel Gorman, Aaron Lu, Netdev,
Alexander Duyck
In-Reply-To: <CAKgT0Uc7wheUjStv5a4BSNv_=-iu1Ttdj9f_10CdR_oc2BhVig@mail.gmail.com>
On Wed, Feb 13, 2019 at 11:42 PM Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Wed, Feb 13, 2019 at 12:42 PM Jann Horn <jannh@google.com> wrote:
> > The basic idea behind ->pagecnt_bias is: If we pre-allocate the maximum
> > number of references that we might need to create in the fastpath later,
> > the bump-allocation fastpath only has to modify the non-atomic bias value
> > that tracks the number of extra references we hold instead of the atomic
> > refcount. The maximum number of allocations we can serve (under the
> > assumption that no allocation is made with size 0) is nc->size, so that's
> > the bias used.
> >
> > However, even when all memory in the allocation has been given away, a
> > reference to the page is still held; and in the `offset < 0` slowpath, the
> > page may be reused if everyone else has dropped their references.
> > This means that the necessary number of references is actually
> > `nc->size+1`.
> >
> > Luckily, from a quick grep, it looks like the only path that can call
> > page_frag_alloc(fragsz=1) is TAP with the IFF_NAPI_FRAGS flag, which
> > requires CAP_NET_ADMIN in the init namespace and is only intended to be
> > used for kernel testing and fuzzing.
>
> Actually that has me somewhat concerned. I wouldn't be surprised if
> most drivers expect the netdev_alloc_frags call to at least output an
> SKB_DATA_ALIGN sized value.
>
> We probably should update __netdev_alloc_frag and __napi_alloc_frag so
> that they will pass fragsz through SKB_DATA_ALIGN.
Do you want to do a separate patch for that? I'd like to not mix
logically separate changes in a single patch, and I also don't have a
good understanding of the alignment concerns here.
> > To test for this issue, put a `WARN_ON(page_ref_count(page) == 0)` in the
> > `offset < 0` path, below the virt_to_page() call, and then repeatedly call
> > writev() on a TAP device with IFF_TAP|IFF_NO_PI|IFF_NAPI_FRAGS|IFF_NAPI,
> > with a vector consisting of 15 elements containing 1 byte each.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> > mm/page_alloc.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 35fdde041f5c..46285d28e43b 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -4675,11 +4675,11 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> > /* Even if we own the page, we do not use atomic_set().
> > * This would break get_page_unless_zero() users.
> > */
> > - page_ref_add(page, size - 1);
> > + page_ref_add(page, size);
> >
> > /* reset page count bias and offset to start of new frag */
> > nc->pfmemalloc = page_is_pfmemalloc(page);
> > - nc->pagecnt_bias = size;
> > + nc->pagecnt_bias = size + 1;
> > nc->offset = size;
> > }
> >
> > @@ -4695,10 +4695,10 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> > size = nc->size;
> > #endif
> > /* OK, page count is 0, we can safely set it */
> > - set_page_count(page, size);
> > + set_page_count(page, size + 1);
> >
> > /* reset page count bias and offset to start of new frag */
> > - nc->pagecnt_bias = size;
> > + nc->pagecnt_bias = size + 1;
> > offset = size - fragsz;
> > }
>
> If we already have to add a constant it might be better to just use
> PAGE_FRAG_CACHE_MAX_SIZE + 1 in all these spots where you are having
> to use "size + 1" instead of "size". That way we can avoid having to
> add a constant to a register value and then program that value.
> instead we can just assign the constant value right from the start.
I doubt that these few instructions make a difference, but sure, I can
send a v2 with that changed.
^ permalink raw reply
* [PATCH net] net: adaptec: starfire: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-14 15:06 UTC (permalink / raw)
To: netdev; +Cc: davem, ionut, yang.wei9, albin_yang
From: Yang Wei <yang.wei9@zte.com.cn>
dev_consume_skb_irq() should be called in intr_handler() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/ethernet/adaptec/starfire.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index 097467f..816540e 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -1390,7 +1390,7 @@ static irqreturn_t intr_handler(int irq, void *dev_instance)
}
}
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
}
np->tx_done_q[np->tx_done].status = 0;
np->tx_done = (np->tx_done + 1) % DONE_Q_SIZE;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
From: Niklas Cassel @ 2019-02-14 15:06 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: Marc Gonzalez, Andrew Lunn, Florian Fainelli, Vinod Koul,
David S Miller, linux-arm-msm, Bjorn Andersson, netdev,
Nori, Sekhar
In-Reply-To: <96271de7-bda1-a86a-a78e-e132bc097efb@ti.com>
On Thu, Feb 14, 2019 at 03:22:28PM +0200, Peter Ujfalusi wrote:
> Hi Niklas,
>
> On 14/02/2019 14.39, Niklas Cassel wrote:
> >>> So, I've rebased your old patch, see attachment.
> >>> I suggest that Peter test it on am335x-evm.
> >>
> >> with the patch + s/rgmii-txid/rgmii-id in the am335x-evmsk.dts ethernet
> >> is working.
> >> I don't have am335x-evm to test, but it has the same PHY as evmsk.
> >>
> >
> > Florian's concern was that this PHY driver looked at "phy-mode" from the
> > perspective of the MAC rather than the PHY.
> > However, if s/rgmii-txid/rgmii-id is the correct fix for am335x-evm,
> > then this means that this PHY driver was just broken.
> >
> > If the driver had misinterpreted the perspective, then the correct
> > fix for am335x-evm would have been s/rgmii-txid/rgmii-rxid.
>
> Not sure if I got this right, but:
> rgmii-id/txid/rxid is the delay mode between PHY and MAC, right?
> on the PHY node it is from the PHY perspective, right?
Yes, from the PHY perspective.
(According to Florian, IIUC, some old PHY drivers were implemented before
it was decided that it is from PHY perspective, rather than from MAC
perspective.)
>
> The errata I have mentioned for am335x say:
> "The reset state of RGMII1_IDMODE (bit 4) and RGMII2_IDMODE (bit 5) in
> the GMII_SEL register enables internal delay mode on the transmit clock
> of the respective RGMII port. The AM335x device does not support
> internal delay mode, so RGMII1_IDMODE and RGMII2_IDMODE must be set to 1b."
>
> If the delay mode on the transmit clock is not working on the am335x,
> then this translate that the rxid needs to be enabled on the PHY side?
IIUC what Florian explained, then either MAC or PHY needs to add delays,
so if the PHY only adds delay on e.g. TX, then the MAC needs to add delay
on RX.
However, in your case, the errata says that your MAC is not capable of
adding a delay on TX, therefore the PHY needs to add a delay on TX.
>
> But then why it worked when only the txid was enabled and rxid was not
> on the PHY side, and why it works if both txid and rxid is enabled?
Because the PHY driver was broken, so the PHY driver always enabled
delays on both TX and RX.
This is how the driver looked before Vinod's change:
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
ret = at803x_enable_rx_delay(phydev);
if (ret < 0)
return ret;
}
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
ret = at803x_enable_tx_delay(phydev);
if (ret < 0)
return ret;
}
Yet, the initial value for this PHY is that both TX and RX delay is
enabled, and since this driver never disabled TX/RX delays,
the TX and RX delays were always enabled, no matter what phy-mode
you specified.
>
> Just tried w/ your patch and setting rgmii-rxid for am335x-evmsk and
> ethernet is not working, it only works w/ rgmii-id (so both tx and rx
> delay is enabled on the PHY side?)
>
> > So considering that this driver seems to be really broken
> > (rather then just inverted perspective),
> > perhaps we can merge the patch I attached in my previous email after all?
> > (Together with a s/rgmii-txid/rgmii-id in the am335x-evmsk.dts.)
>
> at the same time am335x-evm.dts needs to have the same change and most
> likely other boards which uses the same PHY needs to be checked?
>
> PS: sorry for my lack of knowledge on the networking stuff...
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* Re: [net-next, PATCH] net: stmmac: use correct define to get rx timestamp on GMAC4
From: Alexandre Torgue @ 2019-02-14 15:00 UTC (permalink / raw)
To: Jose Abreu, Giuseppe Cavallaro, davem
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <7c03dff4-5185-dbd2-eeb4-867972512f2b@synopsys.com>
Hi Jose
On 2/14/19 3:18 PM, Jose Abreu wrote:
> Hi Alexandre,
>
> On 2/14/2019 2:12 PM, Alexandre Torgue wrote:
>> In dwmac4_wrback_get_rx_timestamp_status we looking for a RX timestamp.
>> For that receive descriptors are handled and so we should use defines
>> related to receive descriptors. It'll no change the functional behavior
>> as RDES3_RDES1_VALID=TDES3_RS1V=BIT(26) but it makes code easier to read.
>>
>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>> index 20299f6..9f062b3 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>> @@ -268,7 +268,7 @@ static int dwmac4_wrback_get_rx_timestamp_status(void *desc, void *next_desc,
>> int ret = -EINVAL;
>>
>> /* Get the status from normal w/b descriptor */
>> - if (likely(p->des3 & TDES3_RS1V)) {
>> + if (likely(p->des3 & RDES3_RDES1_VALID)) {
>
> Shouldn't this also use le32_to_cpu() like bellow ?
I agree. I focused on cosmetic but yes you are right, we have to take
car about endianness as this IP is used by different processors (using
different endianness). I gonna send a v2.
I think dwmac4_rx_check_timestamp have the same kind of issue. Another
patch should be sent for it. no ?
regards
Alex
>
> Thanks,
> Jose Miguel Abreu
>
>> if (likely(le32_to_cpu(p->des1) & RDES1_TIMESTAMP_AVAILABLE)) {
>> int i = 0;
>>
>>
^ permalink raw reply
* Re: [PATCH 2/8] net: ethernet: stmmac: update to support all PHY config for stm32mp157c.
From: Christophe ROULLIER @ 2019-02-14 15:00 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh@kernel.org, davem@davemloft.net, joabreu@synopsys.com,
mark.rutland@arm.com, mcoquelin.stm32@gmail.com, Alexandre TORGUE,
Peppe CAVALLARO, linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190214134040.GA5699@lunn.ch>
On 2/14/19 2:40 PM, Andrew Lunn wrote:
> On Thu, Feb 14, 2019 at 07:45:57AM +0100, Christophe Roullier wrote:
>> @@ -131,19 +185,19 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>> case PHY_INTERFACE_MODE_RGMII:
>> val = SYSCFG_PMCR_ETH_SEL_RGMII;
>> - if (dwmac->int_phyclk)
>> + if (dwmac->eth_clk_sel_reg)
>> val |= SYSCFG_PMCR_ETH_CLK_SEL;
>> pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RGMII\n");
>> break;
>
> Hi Christophe
>
> This code should handle all 4 PHY_INTERFACE_MODE_RGMII* values.
>
> Andrew
>
ReReHi Andrew,
Sorry, I've misunderstood your question ;-)
And you spoke about :
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_TXID:
So in my setup I've only RGMII interface, so I've never tested 3 others
interfaces (_ID, _RXID, _TXID).
So do I need to add cases in my driver ?
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH 7/8] ARM: dts: stm32: Add Ethernet support on stm32h7 SOC and activate it for eval and disco boards
From: Christophe ROULLIER @ 2019-02-14 14:55 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh@kernel.org, davem@davemloft.net, joabreu@synopsys.com,
mark.rutland@arm.com, mcoquelin.stm32@gmail.com, Alexandre TORGUE,
Peppe CAVALLARO, linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190214134414.GB5699@lunn.ch>
On 2/14/19 2:44 PM, Andrew Lunn wrote:
> On Thu, Feb 14, 2019 at 07:46:02AM +0100, Christophe Roullier wrote:
>> + mdio0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "snps,dwmac-mdio";
>> + phy1: ethernet-phy@1 {
>> + reg = <0>;
>> + };
>
> The reg value should be the same as ethernet-phy@X.
>
> Andrew
>> + mdio0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "snps,dwmac-mdio";
>> + phy1: ethernet-phy@1 {
>> + reg = <0>;
>> + };
>
> Here as well.
>
> Andrew
>
Hi Andrew,
Ok I will update.
Thanks.
Christophe
^ permalink raw reply
* [PATCH net] net: 3com: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-14 14:55 UTC (permalink / raw)
To: netdev; +Cc: davem, klassert, anna-maria, bigeasy, yang.wei9, albin_yang
From: Yang Wei <yang.wei9@zte.com.cn>
dev_consume_skb_irq() should be called when skb xmit done. It makes
drop profiles(dropwatch, perf) more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/ethernet/3com/3c515.c | 4 ++--
drivers/net/ethernet/3com/3c59x.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/3com/3c515.c b/drivers/net/ethernet/3com/3c515.c
index b648e3f..808abb6 100644
--- a/drivers/net/ethernet/3com/3c515.c
+++ b/drivers/net/ethernet/3com/3c515.c
@@ -1177,7 +1177,7 @@ static irqreturn_t corkscrew_interrupt(int irq, void *dev_id)
if (inl(ioaddr + DownListPtr) == isa_virt_to_bus(&lp->tx_ring[entry]))
break; /* It still hasn't been processed. */
if (lp->tx_skbuff[entry]) {
- dev_kfree_skb_irq(lp->tx_skbuff[entry]);
+ dev_consume_skb_irq(lp->tx_skbuff[entry]);
lp->tx_skbuff[entry] = NULL;
}
dirty_tx++;
@@ -1192,7 +1192,7 @@ static irqreturn_t corkscrew_interrupt(int irq, void *dev_id)
#ifdef VORTEX_BUS_MASTER
if (status & DMADone) {
outw(0x1000, ioaddr + Wn7_MasterStatus); /* Ack the event. */
- dev_kfree_skb_irq(lp->tx_skb); /* Release the transferred buffer */
+ dev_consume_skb_irq(lp->tx_skb); /* Release the transferred buffer */
netif_wake_queue(dev);
}
#endif
diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
index 40f421d..1470514 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -2307,7 +2307,7 @@ _vortex_interrupt(int irq, struct net_device *dev)
dma_unmap_single(vp->gendev, vp->tx_skb_dma, (vp->tx_skb->len + 3) & ~3, DMA_TO_DEVICE);
pkts_compl++;
bytes_compl += vp->tx_skb->len;
- dev_kfree_skb_irq(vp->tx_skb); /* Release the transferred buffer */
+ dev_consume_skb_irq(vp->tx_skb); /* Release the transferred buffer */
if (ioread16(ioaddr + TxFree) > 1536) {
/*
* AKPM: FIXME: I don't think we need this. If the queue was stopped due to
@@ -2449,7 +2449,7 @@ _boomerang_interrupt(int irq, struct net_device *dev)
#endif
pkts_compl++;
bytes_compl += skb->len;
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
vp->tx_skbuff[entry] = NULL;
} else {
pr_debug("boomerang_interrupt: no skb!\n");
--
2.7.4
^ permalink raw reply related
* [PATCH net] net: arc_emac: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-14 14:53 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, yang.wei9, albin_yang
From: Yang Wei <yang.wei9@zte.com.cn>
dev_consume_skb_irq() should be called in arc_emac_tx_clean() when
skb xmit done. It makes drop profiles(dropwatch, perf) more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/ethernet/arc/emac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
index 4406325..ff3d685 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -148,7 +148,7 @@ static void arc_emac_tx_clean(struct net_device *ndev)
dma_unmap_len(tx_buff, len), DMA_TO_DEVICE);
/* return the sk_buff to system */
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
txbd->data = 0;
txbd->info = 0;
--
2.7.4
^ permalink raw reply related
* [PATCH net] net: packetengines: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-14 14:52 UTC (permalink / raw)
To: netdev; +Cc: davem, yang.wei9, albin_yang
From: Yang Wei <yang.wei9@zte.com.cn>
dev_consume_skb_irq() should be called when skb xmit done. It makes
drop profiles(dropwatch, perf) more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/ethernet/packetengines/hamachi.c | 2 +-
drivers/net/ethernet/packetengines/yellowfin.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/packetengines/hamachi.c b/drivers/net/ethernet/packetengines/hamachi.c
index c9529c2..eee883a 100644
--- a/drivers/net/ethernet/packetengines/hamachi.c
+++ b/drivers/net/ethernet/packetengines/hamachi.c
@@ -1337,7 +1337,7 @@ static irqreturn_t hamachi_interrupt(int irq, void *dev_instance)
leXX_to_cpu(hmp->tx_ring[entry].addr),
skb->len,
PCI_DMA_TODEVICE);
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
hmp->tx_skbuff[entry] = NULL;
}
hmp->tx_ring[entry].status_n_length = 0;
diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
index 54224d1..6f8d658 100644
--- a/drivers/net/ethernet/packetengines/yellowfin.c
+++ b/drivers/net/ethernet/packetengines/yellowfin.c
@@ -925,7 +925,7 @@ static irqreturn_t yellowfin_interrupt(int irq, void *dev_instance)
/* Free the original skb. */
pci_unmap_single(yp->pci_dev, le32_to_cpu(yp->tx_ring[entry].addr),
skb->len, PCI_DMA_TODEVICE);
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
yp->tx_skbuff[entry] = NULL;
}
if (yp->tx_full &&
@@ -983,7 +983,7 @@ static irqreturn_t yellowfin_interrupt(int irq, void *dev_instance)
pci_unmap_single(yp->pci_dev,
yp->tx_ring[entry<<1].addr, skb->len,
PCI_DMA_TODEVICE);
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
yp->tx_skbuff[entry] = 0;
/* Mark status as empty. */
yp->tx_status[entry].tx_errs = 0;
--
2.7.4
^ permalink raw reply related
* Re: KASAN: use-after-free Read in sctp_outq_tail
From: Xin Long @ 2019-02-14 14:52 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: syzbot, davem, LKML, linux-sctp, network dev, Neil Horman,
syzkaller-bugs, Vlad Yasevich
In-Reply-To: <20190214143911.GL10665@localhost.localdomain>
On Thu, Feb 14, 2019 at 10:39 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Thu, Feb 14, 2019 at 12:17:45PM -0200, Marcelo Ricardo Leitner wrote:
> > On Thu, Feb 14, 2019 at 10:03:30PM +0800, Xin Long wrote:
> > > On Wed, Feb 13, 2019 at 9:52 PM Marcelo Ricardo Leitner
> > > <marcelo.leitner@gmail.com> wrote:
> > > >
> > > > On Wed, Feb 13, 2019 at 12:35:56PM +0800, Xin Long wrote:
> > > > > On Wed, Feb 13, 2019 at 4:00 AM syzbot
> > > > > <syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com> wrote:
> > > > > >
> > > > > > Hello,
> > > > > >
> > > > > > syzbot found the following crash on:
> > > > > >
> > > > > > HEAD commit: d4104460aec1 Add linux-next specific files for 20190211
> > > > > > git tree: linux-next
> > > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=14140124c00000
> > > > > > kernel config: https://syzkaller.appspot.com/x/.config?x=c8a112d3b0d6719b
> > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=7823fa3f3e2d69341ea8
> > > > > > compiler: gcc (GCC) 9.0.0 20181231 (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+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com
> > > > > >
> > > > > > ==================================================================
> > > > > > BUG: KASAN: use-after-free in list_add_tail include/linux/list.h:93 [inline]
> > > > > > BUG: KASAN: use-after-free in sctp_outq_tail_data net/sctp/outqueue.c:105
> > > > > > [inline]
> > > > > > BUG: KASAN: use-after-free in sctp_outq_tail+0x816/0x930
> > > > > > net/sctp/outqueue.c:313
> > > > > > Read of size 8 at addr ffff88807b19a7b8 by task syz-executor.0/30745
> > > > > I think https://patchwork.ozlabs.org/patch/1040500/ will fix this.
> > > >
> > > > I don't think so. Seems it will switch from use-after-free to NULL deref
> > > > instead with that patch.
> > > It will allocate ext for the stream if its ext is NULL in
> > > sctp_sendmsg_to_asoc(), the new data will work fine. As for
> >
> > Ehh, right!
> >
> > > the old data on the surplus streams, it has been dropped in
> > > sctp_stream_outq_migrate().
> >
> > Yep.
> >
> > >
> > > >
> > > > > > CPU: 1 PID: 30745 Comm: syz-executor.0 Not tainted 5.0.0-rc5-next-20190211
> > > > > > #32
> > > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > > > Google 01/01/2011
> > > > > > Call Trace:
> > > > > > __dump_stack lib/dump_stack.c:77 [inline]
> > > > > > dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> > > > > > print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
> > > > > > kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> > > > > > __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
> > > > > > list_add_tail include/linux/list.h:93 [inline]
> > > > > > sctp_outq_tail_data net/sctp/outqueue.c:105 [inline]
> > > > > > sctp_outq_tail+0x816/0x930 net/sctp/outqueue.c:313
> > > > > > sctp_cmd_send_msg net/sctp/sm_sideeffect.c:1109 [inline]
> > > > > > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1784 [inline]
> > > > > > sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> > > > > > sctp_do_sm+0x68e/0x5380 net/sctp/sm_sideeffect.c:1191
> > > > > > sctp_primitive_SEND+0xa0/0xd0 net/sctp/primitive.c:178
> > > > > > sctp_sendmsg_to_asoc+0xa63/0x17b0 net/sctp/socket.c:1955
> > > >
> > > > sctp_sendmsg_to_asoc()
> > > > ...
> > > > if (sinfo->sinfo_stream >= asoc->stream.outcnt) {
> > > > err = -EINVAL;
> > > > goto err;
> > > > }
> > > >
> > > > if (unlikely(!SCTP_SO(&asoc->stream, sinfo->sinfo_stream)->ext)) {
> > > > ...
> > > >
> > > > It should have aborted even if an old ->ext was still there because
> > > > outcnt is correctly updated. So somehow outcnt was wrong here.
> > > >
> > > > sctp_stream_init()
> > > > ...
> > > > /* Filter out chunks queued on streams that won't exist anymore */
> > > > sched->unsched_all(stream);
> > > > sctp_stream_outq_migrate(stream, NULL, outcnt); <--- [A]
> > > > sched->sched_all(stream);
> > > >
> > > > ret = sctp_stream_alloc_out(stream, outcnt, gfp); <--- [B]
> > > > if (ret)
> > > > goto out;
> > > >
> > > > stream->outcnt = outcnt; <--- [C]
> > > > ...
> > > >
> > > > We have a problem here because [A] is freeing ->ext, but [B] can fail (ENOMEM),
> > > > which would lead it to not update outcnt in [C] even after the
> > > > changes already performed in [A].
> > > >
> > > > It should handle the freeing of ->ext in sctp_stream_alloc_out()
> > > > instead, or allocate the flexarray earlier (so it can bail out before
> > > > freeing stuff).
> > > Agree that it's better to do:
> > > sched->unsched_all(stream);
> > > sctp_stream_outq_migrate(stream, NULL, outcnt);
> > > sched->sched_all(stream);
> > > after the flexarray allocation.
> > >
> > > Just sctp_stream_alloc_out() can not be used here anymore, as
> > > stream->out will be set in it.
> >
> > What about carving out a sctp_stream_init_out() ? Like
> >
> > struct flex_array *new_out;
> >
> > /* just allocate it, nothing more */
> > new_out = sctp_stream_alloc_out(outcnt, gfp);
> > if (!new_out)
> > goto out;
> >
> > /* Filter out chunks queued on streams that won't exist anymore */
> > sched->unsched_all(stream);
> > sctp_stream_outq_migrate(stream, NULL, outcnt);
> > sched->sched_all(stream);
> >
> > /* initialization stuff, and it can't fail anymore */
> > sctp_stream_init_out(stream, outcnt, new_out);
> >
> > This may hurt a bit more with the genradix migration, but then we
> > don't end up dropping data for nothing.
>
> Reviewing calls to this function, if this allocation fails it will
> either cause the asoc to be terminated or sendmsg error. So data loss
> is not really an issue here.
>
sctp_stream_init+0xbc/0x410 net/sctp/stream.c:139
sctp_process_init+0x21c3/0x2b20 net/sctp/sm_make_chunk.c:2466
sctp_cmd_process_init net/sctp/sm_sideeffect.c:682 [inline]
sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1410 [inline]
on this path, seems that it won't terminate the asoc nor report a sending error.
^ permalink raw reply
* [PATCH net] net: xilinx: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-14 14:50 UTC (permalink / raw)
To: netdev
Cc: davem, michal.simek, anirudh, John.Linn, radhey.shyam.pandey,
andrew, yang.wei9, albin_yang
From: Yang Wei <yang.wei9@zte.com.cn>
dev_consume_skb_irq() should be called when skb xmit done. It makes
drop profiles(dropwatch, perf) more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 2 +-
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +-
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 15bb058..44efffb 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -630,7 +630,7 @@ static void temac_start_xmit_done(struct net_device *ndev)
dma_unmap_single(ndev->dev.parent, cur_p->phys, cur_p->len,
DMA_TO_DEVICE);
if (cur_p->app4)
- dev_kfree_skb_irq((struct sk_buff *)cur_p->app4);
+ dev_consume_skb_irq((struct sk_buff *)cur_p->app4);
cur_p->app0 = 0;
cur_p->app1 = 0;
cur_p->app2 = 0;
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 0789d8a..ec7e7ec 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -595,7 +595,7 @@ static void axienet_start_xmit_done(struct net_device *ndev)
(cur_p->cntrl & XAXIDMA_BD_CTRL_LENGTH_MASK),
DMA_TO_DEVICE);
if (cur_p->app4)
- dev_kfree_skb_irq((struct sk_buff *)cur_p->app4);
+ dev_consume_skb_irq((struct sk_buff *)cur_p->app4);
/*cur_p->phys = 0;*/
cur_p->app0 = 0;
cur_p->app1 = 0;
diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 639e3e9..b03a417 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -581,7 +581,7 @@ static void xemaclite_tx_handler(struct net_device *dev)
return;
dev->stats.tx_bytes += lp->deferred_skb->len;
- dev_kfree_skb_irq(lp->deferred_skb);
+ dev_consume_skb_irq(lp->deferred_skb);
lp->deferred_skb = NULL;
netif_trans_update(dev); /* prevent tx timeout */
netif_wake_queue(dev);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 2/8] net: ethernet: stmmac: update to support all PHY config for stm32mp157c.
From: Christophe ROULLIER @ 2019-02-14 14:48 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh@kernel.org, davem@davemloft.net, joabreu@synopsys.com,
mark.rutland@arm.com, mcoquelin.stm32@gmail.com, Alexandre TORGUE,
Peppe CAVALLARO, linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190214134040.GA5699@lunn.ch>
On 2/14/19 2:40 PM, Andrew Lunn wrote:
> On Thu, Feb 14, 2019 at 07:45:57AM +0100, Christophe Roullier wrote:
>> @@ -131,19 +185,19 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>> case PHY_INTERFACE_MODE_RGMII:
>> val = SYSCFG_PMCR_ETH_SEL_RGMII;
>> - if (dwmac->int_phyclk)
>> + if (dwmac->eth_clk_sel_reg)
>> val |= SYSCFG_PMCR_ETH_CLK_SEL;
>> pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RGMII\n");
>> break;
>
> Hi Christophe
>
> This code should handle all 4 PHY_INTERFACE_MODE_RGMII* values.
>
> Andrew
>
Hi Andrew,
For RGMII we are supporting 3 modes:
- normal mode (with PHY with quartz)
- Phy wo crystal and phy is clocking with PLL to 25Mhz.
- other mode where we used PLL from RCC (125Mhz) to clock Ethernet
IP (save one pin "ETH_CK125")
In driver source code I put table to sum-up all configs supported for
each phy mode:
+ * Below table summarizes the clock requirement and clock sources for
+ * supported phy interface modes.
+ *
__________________________________________________________________________
+ *|PHY_MODE | Normal | PHY wo crystal| PHY wo crystal |No 125Mhz
from PHY|
+ *| | | 25MHz | 50MHz |
|
+ *
---------------------------------------------------------------------------
+ *| MII | - | eth-ck | n/a | n/a |
+ *| | | | | |
+ *
---------------------------------------------------------------------------
+ *| GMII | - | eth-ck | n/a | n/a |
+ *| | | | | |
+ *
---------------------------------------------------------------------------
+ *| RGMII | - | eth-ck | n/a | eth-ck (no pin) |
+ *| | | | |
st,eth_clk_sel |
+ *
---------------------------------------------------------------------------
+ *| RMII | - | eth-ck | eth-ck | n/a |
+ *| | | | st,eth_ref_clk_sel | |
+ *
---------------------------------------------------------------------------
Regards,
Christophe
^ permalink raw reply
* Re: [patch net-next] lib: objagg: fix handling of object with 0 users when assembling hints
From: Ido Schimmel @ 2019-02-14 14:47 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, mlxsw
In-Reply-To: <20190214143907.3275-1-jiri@resnulli.us>
On Thu, Feb 14, 2019 at 03:39:07PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> It is possible that there might be an originally parent object with 0
> direct users that is in hints no longer considered as parent. Then the
> weight of this object is 0 and current code ignores him. That's why the
> total amount of hint objects might be lower than for the original
> objagg and WARN_ON is hit. Fix this be considering 0 weight valid.
>
> Fixes: 9069a3817d82 ("lib: objagg: implement optimization hints assembly and use hints for object creation")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
^ permalink raw reply
* [PATCH net] net: i825xx: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-14 14:45 UTC (permalink / raw)
To: netdev; +Cc: davem, yang.wei9, albin_yang
From: Yang Wei <yang.wei9@zte.com.cn>
dev_consume_skb_irq() should be called in i596_interrupt() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/ethernet/i825xx/lib82596.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/i825xx/lib82596.c b/drivers/net/ethernet/i825xx/lib82596.c
index 2f7ae11..1274ad2 100644
--- a/drivers/net/ethernet/i825xx/lib82596.c
+++ b/drivers/net/ethernet/i825xx/lib82596.c
@@ -1194,7 +1194,7 @@ static irqreturn_t i596_interrupt(int irq, void *dev_id)
dma_unmap_single(dev->dev.parent,
tx_cmd->dma_addr,
skb->len, DMA_TO_DEVICE);
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
tx_cmd->cmd.command = 0; /* Mark free */
break;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox