mbox series

[0/7] net: thunderx: implement DMAC filtering support

Message ID 20180327150736.10718-1-Vadim.Lomovtsev@caviumnetworks.com
Headers show
Series net: thunderx: implement DMAC filtering support | expand

Message

Vadim Lomovtsev March 27, 2018, 3:07 p.m. UTC
From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>

By default CN88XX BGX accepts all incoming multicast and broadcast
packets and filtering is disabled. The nic driver doesn't provide
an ability to change such behaviour.

This series is to implement DMAC filtering management for CN88XX
nic driver allowing user to enable/disable filtering and configure
specific MAC addresses to filter traffic.

Vadim Lomovtsev (7):
  net: thunderx: move filter register related macro into proper place
  net: thunderx: add MAC address filter tracking for LMAC
  net: thunderx: add multicast filter management support
  net: thunderx: add new messages for handle ndo_set_rx_mode callback
  net: thunderx: add XCAST messages handlers for PF
  net: thunderx: add workqueue control structures for handle
    ndo_set_rx_mode request
  net: thunderx: add ndo_set_rx_mode callback implementation for VF

 drivers/net/ethernet/cavium/thunder/nic.h         |  29 ++++
 drivers/net/ethernet/cavium/thunder/nic_main.c    |  45 ++++-
 drivers/net/ethernet/cavium/thunder/nicvf_main.c  | 108 +++++++++++-
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 196 ++++++++++++++++++++--
 drivers/net/ethernet/cavium/thunder/thunder_bgx.h |  17 +-
 5 files changed, 366 insertions(+), 29 deletions(-)

Comments

David Miller March 27, 2018, 5:28 p.m. UTC | #1
From: Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com>
Date: Tue, 27 Mar 2018 08:07:29 -0700

> From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
> 
> By default CN88XX BGX accepts all incoming multicast and broadcast
> packets and filtering is disabled. The nic driver doesn't provide
> an ability to change such behaviour.
> 
> This series is to implement DMAC filtering management for CN88XX
> nic driver allowing user to enable/disable filtering and configure
> specific MAC addresses to filter traffic.

This doesn't even compile:

drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function ‘bgx_lmac_save_filter’:
drivers/net/ethernet/cavium/thunder/thunder_bgx.c:286:3: warning: ‘return’ with no value, in function returning non-void [-Wreturn-type]
   return;
   ^~~~~~
drivers/net/ethernet/cavium/thunder/thunder_bgx.c:281:12: note: declared here
 static int bgx_lmac_save_filter(struct lmac *lmac, u64 dmac, u8 vf_id)
            ^~~~~~~~~~~~~~~~~~~~
In file included from drivers/net/ethernet/cavium/thunder/nic.h:15:0,
                 from drivers/net/ethernet/cavium/thunder/thunder_bgx.c:21:
drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function ‘bgx_set_dmac_cam_filter_mac’:
drivers/net/ethernet/cavium/thunder/thunder_bgx.h:61:38: warning: left shift count >= width of type [-Wshift-count-overflow]
 #define  RX_DMACX_CAM_LMACID(x)   (x << 49)
                                      ^
drivers/net/ethernet/cavium/thunder/thunder_bgx.c:324:8: note: in expansion of macro ‘RX_DMACX_CAM_LMACID’
  cfg = RX_DMACX_CAM_LMACID(lmacid & LMAC_ID_MASK) |
        ^~~~~~~~~~~~~~~~~~~
Vadim Lomovtsev March 28, 2018, 8:47 a.m. UTC | #2
Hi David,

Thanks for feedback.

On Tue, Mar 27, 2018 at 01:28:22PM -0400, David Miller wrote:
> From: Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com>
> Date: Tue, 27 Mar 2018 08:07:29 -0700
> 
> > From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
> > 
> > By default CN88XX BGX accepts all incoming multicast and broadcast
> > packets and filtering is disabled. The nic driver doesn't provide
> > an ability to change such behaviour.
> > 
> > This series is to implement DMAC filtering management for CN88XX
> > nic driver allowing user to enable/disable filtering and configure
> > specific MAC addresses to filter traffic.
> 
> This doesn't even compile:
> 
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function ‘bgx_lmac_save_filter’:
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:286:3: warning: ‘return’ with no value, in function returning non-void [-Wreturn-type]
>    return;
>    ^~~~~~
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:281:12: note: declared here
>  static int bgx_lmac_save_filter(struct lmac *lmac, u64 dmac, u8 vf_id)
>             ^~~~~~~~~~~~~~~~~~~~
> In file included from drivers/net/ethernet/cavium/thunder/nic.h:15:0,
>                  from drivers/net/ethernet/cavium/thunder/thunder_bgx.c:21:
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function ‘bgx_set_dmac_cam_filter_mac’:
> drivers/net/ethernet/cavium/thunder/thunder_bgx.h:61:38: warning: left shift count >= width of type [-Wshift-count-overflow]
>  #define  RX_DMACX_CAM_LMACID(x)   (x << 49)
>                                       ^
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:324:8: note: in expansion of macro ‘RX_DMACX_CAM_LMACID’
>   cfg = RX_DMACX_CAM_LMACID(lmacid & LMAC_ID_MASK) |
>         ^~~~~~~~~~~~~~~~~~~

Will update and repost series as v2.
Do you have any other comments ?

WBR,
Vadim
Vadim Lomovtsev Feb. 20, 2019, 11:02 a.m. UTC | #3
The ThunderX CN88XX NIC Virtual Function driver uses mailbox interface
to communicate to physical function driver. Each of VF has it's own pair
of mailbox registers to read from and write to. The mailbox registers
has no protection from possible races, so it has to be implemented
at software side.

After long term testing by loop of 'ip link set <ifname> up/down'
command it was found that there are two possible scenarios when
race condition appears:
 1. VF receives link change message from PF and VF send RX mode
configuration message to PF in the same time from separate thread.
 2. PF receives RX mode configuration from VF and in the same time,
in separate thread PF detects link status change and sends appropriate
message to particular VF.

Both cases leads to mailbox data to be rewritten, NIC VF messaging control
data to be updated incorrectly and communication sequence gets broken.

This patch series is to address race condition with VF & PF communication.

Changes:
v1 -> v2
 - 0000: correct typo in cover letter subject: 'betwen' -> 'between';
 - move link state polling request task from pf to vf 
   instead of cheking status of mailbox irq;
v2 -> v3
 - 0003: change return type of nicvf_send_cfg_done() function
   from int to void;
 - 0007: update subject and remove unused variable 'netdev'
   from nicvf_link_status_check_task() function;

Vadim Lomovtsev (8):
  net: thunderx: correct typo in macro name
  net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs
    to private for each of them.
  net: thunderx: make CFG_DONE message to run through generic send-ack
    sequence
  net: thunderx: add nicvf_send_msg_to_pf result check for
    set_rx_mode_task
  net: thunderx: rework xcast message structure to make it fit into 64
    bit
  net: thunderx: add mutex to protect mailbox from concurrent calls for
    same VF
  net: thunderx: add LINK_CHANGE message handler at nicpf
  net: thunderx: remove link change polling code and info from nicpf

 drivers/net/ethernet/cavium/thunder/nic.h     |  14 +-
 .../net/ethernet/cavium/thunder/nic_main.c    | 149 ++++++------------
 .../net/ethernet/cavium/thunder/nicvf_main.c  | 130 ++++++++++-----
 .../net/ethernet/cavium/thunder/thunder_bgx.c |   2 +-
 .../net/ethernet/cavium/thunder/thunder_bgx.h |   2 +-
 5 files changed, 144 insertions(+), 153 deletions(-)
Vadim Lomovtsev Feb. 20, 2019, 11:19 a.m. UTC | #4
sorry for occasionally reply to old thread.

On Wed, Feb 20, 2019 at 11:02:42AM +0000, Vadim Lomovtsev wrote:
> The ThunderX CN88XX NIC Virtual Function driver uses mailbox interface
> to communicate to physical function driver. Each of VF has it's own pair
> of mailbox registers to read from and write to. The mailbox registers
> has no protection from possible races, so it has to be implemented
> at software side.
> 
> After long term testing by loop of 'ip link set <ifname> up/down'
> command it was found that there are two possible scenarios when
> race condition appears:
>  1. VF receives link change message from PF and VF send RX mode
> configuration message to PF in the same time from separate thread.
>  2. PF receives RX mode configuration from VF and in the same time,
> in separate thread PF detects link status change and sends appropriate
> message to particular VF.
> 
> Both cases leads to mailbox data to be rewritten, NIC VF messaging control
> data to be updated incorrectly and communication sequence gets broken.
> 
> This patch series is to address race condition with VF & PF communication.
> 
> Changes:
> v1 -> v2
>  - 0000: correct typo in cover letter subject: 'betwen' -> 'between';
>  - move link state polling request task from pf to vf 
>    instead of cheking status of mailbox irq;
> v2 -> v3
>  - 0003: change return type of nicvf_send_cfg_done() function
>    from int to void;
>  - 0007: update subject and remove unused variable 'netdev'
>    from nicvf_link_status_check_task() function;
> 
> Vadim Lomovtsev (8):
>   net: thunderx: correct typo in macro name
>   net: thunderx: replace global nicvf_rx_mode_wq work queue for all VFs
>     to private for each of them.
>   net: thunderx: make CFG_DONE message to run through generic send-ack
>     sequence
>   net: thunderx: add nicvf_send_msg_to_pf result check for
>     set_rx_mode_task
>   net: thunderx: rework xcast message structure to make it fit into 64
>     bit
>   net: thunderx: add mutex to protect mailbox from concurrent calls for
>     same VF
>   net: thunderx: add LINK_CHANGE message handler at nicpf
>   net: thunderx: remove link change polling code and info from nicpf
> 
>  drivers/net/ethernet/cavium/thunder/nic.h     |  14 +-
>  .../net/ethernet/cavium/thunder/nic_main.c    | 149 ++++++------------
>  .../net/ethernet/cavium/thunder/nicvf_main.c  | 130 ++++++++++-----
>  .../net/ethernet/cavium/thunder/thunder_bgx.c |   2 +-
>  .../net/ethernet/cavium/thunder/thunder_bgx.h |   2 +-
>  5 files changed, 144 insertions(+), 153 deletions(-)
> 
> -- 
> 2.17.2
David Miller Feb. 22, 2019, 7:44 p.m. UTC | #5
From: Vadim Lomovtsev <vlomovtsev@marvell.com>
Date: Wed, 20 Feb 2019 11:02:42 +0000

> The ThunderX CN88XX NIC Virtual Function driver uses mailbox interface
> to communicate to physical function driver. Each of VF has it's own pair
> of mailbox registers to read from and write to. The mailbox registers
> has no protection from possible races, so it has to be implemented
> at software side.
> 
> After long term testing by loop of 'ip link set <ifname> up/down'
> command it was found that there are two possible scenarios when
> race condition appears:
>  1. VF receives link change message from PF and VF send RX mode
> configuration message to PF in the same time from separate thread.
>  2. PF receives RX mode configuration from VF and in the same time,
> in separate thread PF detects link status change and sends appropriate
> message to particular VF.
> 
> Both cases leads to mailbox data to be rewritten, NIC VF messaging control
> data to be updated incorrectly and communication sequence gets broken.
> 
> This patch series is to address race condition with VF & PF communication.
> 
> Changes:
> v1 -> v2
>  - 0000: correct typo in cover letter subject: 'betwen' -> 'between';
>  - move link state polling request task from pf to vf 
>    instead of cheking status of mailbox irq;
> v2 -> v3
>  - 0003: change return type of nicvf_send_cfg_done() function
>    from int to void;
>  - 0007: update subject and remove unused variable 'netdev'
>    from nicvf_link_status_check_task() function;

Series applied, thanks.