lkml.org 
[lkml]   [2024]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    Date
    From
    SubjectRe: [PATCH net-next v4 05/12] net: ethernet: oa_tc6: implement error interrupts unmasking
    > It could be the performance increase comes from two places:
    >
    > 1) Spending time and bus bandwidth dealing with the buffer overflow
    > interrupt
    >
    > 2) Printing out the serial port.
    >
    > Please could you benchmark a few things:
    >
    > 1) Remove the printk("Receive buffer overflow error"), but otherwise
    > keep the code the same. That will give us an idea how much the serial
    > port matters.
    >
    > 2) Disable only the RX buffer overflow interrupt
    >
    > 3) Disable all error interrupts.
    >



    Test setup
    - Server side -
    PC with a lan8670 usb eval board running a http server that serves
    a 10MB binary blob. The http server is just python3 -m http.server

    - Client side -
    iMX8mn board (quad core arm64) with lan8650 mac-phy (25MHz spi)
    running curl to download the blob from the server and writing to
    a ramfs (ddr4 1.xGHz, so should not be a bottleneck)

    Below are a collection of samples of different test runs. All of the
    test runs have run a minor patch for hw reset (else nothing works for me).
    Using curl is not the most scientific approach here, but iperf has
    not exposed any problems for me earlier with overflows.
    So sticking with curl since it's easy and definetly gets the overflows.

    n | name | min | avg | max | rx dropped | samples
    1 | no mod | 827K | 846K | 891K | 945 | 5
    2 | no log | 711K | 726K | 744K | 562 | 5
    3 | less irq | 815K | 833K | 846K | N/A | 5
    4 | no irq | 914K | 924K | 931K | N/A | 5
    5 | simple | 857K | 868K | 879K | 615 | 5

    Description of each scenario

    1 - no mod
    So this just runs a hw reset to get the chip working (described in earlier posts)

    2 - no log
    This scenario just removes the logging when handling the irq state
    --- a/drivers/net/ethernet/oa_tc6.c
    +++ b/drivers/net/ethernet/oa_tc6.c
    @@ -688,8 +688,6 @@ static int oa_tc6_process_extended_status(struct oa_tc6 *tc6)
    if (FIELD_GET(STATUS0_RX_BUFFER_OVERFLOW_ERROR, value)) {
    tc6->rx_buf_overflow = true;
    oa_tc6_cleanup_ongoing_rx_skb(tc6);
    - net_err_ratelimited("%s: Receive buffer overflow error\n",
    - tc6->netdev->name);
    return -EAGAIN;
    }
    if (FIELD_GET(STATUS0_TX_PROTOCOL_ERROR, value)) {
    3 - less irq
    This scenario disables the overflow interrupt but keeps the others

    --- a/drivers/net/ethernet/oa_tc6.c
    +++ b/drivers/net/ethernet/oa_tc6.c
    @@ -625,10 +625,10 @@ static int oa_tc6_unmask_macphy_error_interrupts(struct oa_tc6 *tc6)
    return ret;

    regval &= ~(INT_MASK0_TX_PROTOCOL_ERR_MASK |
    - INT_MASK0_RX_BUFFER_OVERFLOW_ERR_MASK |
    INT_MASK0_LOSS_OF_FRAME_ERR_MASK |
    INT_MASK0_HEADER_ERR_MASK);

    + regval |= INT_MASK0_RX_BUFFER_OVERFLOW_ERR_MASK;
    return oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
    }

    4 - no irq
    This scenario disables all imask0 interrupts with the following change
    diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
    index 9f17f3712137..88a9c6ccb37a 100644
    --- a/drivers/net/ethernet/oa_tc6.c
    +++ b/drivers/net/ethernet/oa_tc6.c
    @@ -624,12 +624,7 @@ static int oa_tc6_unmask_macphy_error_interrupts(struct oa_tc6 *tc6)
    if (ret)
    return ret;

    - regval &= ~(INT_MASK0_TX_PROTOCOL_ERR_MASK |
    - INT_MASK0_RX_BUFFER_OVERFLOW_ERR_MASK |
    - INT_MASK0_LOSS_OF_FRAME_ERR_MASK |
    - INT_MASK0_HEADER_ERR_MASK);
    -
    - return oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, regval);
    + return oa_tc6_write_register(tc6, OA_TC6_REG_INT_MASK0, (u32)-1);
    }

    static int oa_tc6_enable_data_transfer(struct oa_tc6 *tc6)

    5 - simple
    This keeps the interrupt but does not log or drop the socket buffer on irq
    Moving the rx dropped increment here makes it more of a irq counter I guess,
    so maybe not relevant.

    diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
    index 9f17f3712137..cbc20a725ad0 100644
    --- a/drivers/net/ethernet/oa_tc6.c
    +++ b/drivers/net/ethernet/oa_tc6.c
    @@ -687,10 +687,7 @@ static int oa_tc6_process_extended_status(struct oa_tc6 *tc6)

    if (FIELD_GET(STATUS0_RX_BUFFER_OVERFLOW_ERROR, value)) {
    tc6->rx_buf_overflow = true;
    - oa_tc6_cleanup_ongoing_rx_skb(tc6);
    - net_err_ratelimited("%s: Receive buffer overflow error\n",
    - tc6->netdev->name);
    - return -EAGAIN;
    + tc6->netdev->stats.rx_dropped++;
    }
    if (FIELD_GET(STATUS0_TX_PROTOCOL_ERROR, value)) {
    netdev_err(tc6->netdev, "Transmit protocol error\n");

    - postamble -

    Removing the logging made things considerably slower which probably
    indicates that there is a timing dependent behaviour in the driver.

    I have a hard time explaining why there is a throughput difference
    between scenario 3 and 4 since I did not get the logs that any of the
    other interrupts happened.
    Maybe the irq handling adds some unexpected context switching overhead.

    My recommendation going forward would be to disable the rx buffer
    overlow interrupt and removing any code related to handling of it.

    R

    \
     
     \ /
      Last update: 2024-05-27 18:07    [W:2.565 / U:0.064 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site