Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On Wed, Mar 25, 2020 at 12:53:18AM +0100, Ilya Maximets wrote:
> On 3/6/20 5:11 PM, Ilya Maximets wrote:
> > On 3/4/20 7:09 PM, William Tu wrote:
> >> On Fri, Feb 28, 2020 at 12:12 AM Ilya Maximets wrote:
> >>>
> >>> On 2/27/20 8:52 PM, William Tu wrote:
> On Thu, Feb 27, 2020 at 11:10 AM William Tu wrote:
> >
> > On Thu, Feb 27, 2020 at 9:54 AM Ilya Maximets
> > wrote:
> >>
> >> On 2/27/20 6:51 PM, William Tu wrote:
> >>> On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets
> >>> wrote:
>
> On 2/27/20 6:30 PM, William Tu wrote:
> > The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
> > interrupt mode. This is similar to 'type=afxdp', except that the
> > is_pmd field is set to false. As a result, the packet processing
> > is handled by main thread, not pmd thread. This avoids burning
> > the CPU to always 100% when there is no traffic.
> >
> > Tested-at:
> > https://travis-ci.org/williamtu/ovs-travis/builds/655885506
> > Signed-off-by: William Tu
> > ---
> > NEWS | 3 +++
> > lib/netdev-afxdp.c| 20 +++-
> > lib/netdev-linux.c| 20
> > lib/netdev-provider.h | 1 +
> > lib/netdev.c | 1 +
> > tests/system-afxdp.at | 23 +++
> > 6 files changed, 67 insertions(+), 1 deletion(-)
> >
> > diff --git a/NEWS b/NEWS
> > index f62ef1f47ea8..594c55dc11d6 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -4,6 +4,9 @@ Post-v2.13.0
> > - OpenFlow:
> > * The OpenFlow ofp_desc/serial_num may now be configured by
> > setting the
> > value of other-config:dp-sn in the Bridge table.
> > + - AF_XDP:
> > + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save
> > CPU cycles
> > + by enabling interrupt mode.
> >
> >
> > v2.13.0 - 14 Feb 2020
> > diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> > index 482400d8d135..cd2c7c381139 100644
> > --- a/lib/netdev-afxdp.c
> > +++ b/lib/netdev-afxdp.c
> > @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
> > );
> > };
> >
> > +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd
> > mode. */
> > +static bool
> > +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
> > +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
> > +}
> > +
> > #ifdef HAVE_XDP_NEED_WAKEUP
> > static inline void
> > xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> > @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev
> > *netdev, int qid,
> > struct netdev_linux *dev;
> > int ret;
> >
> > -if (concurrent_txq) {
> > +/* Lock is required when mixing afxdp pmd and nonpmd mode.
> > + * ex: one device is created 'afxdp', the other is
> > 'afxdp-nonpmd'.
> > + */
> > +if (concurrent_txq || (nonpmd_cnt != 0)) {
>
> I'm confused about this change. Are you expecting same device being
> opened twice with different netdev type? Database should not allow
> this.
> >>>
> >>> Not the same device.
> >>>
> >>> For example, dev1 opened as 'afxdp', and dev2 opened as
> >>> 'afxdp-nonpmd'.
> >>> When dev2 receives a packet and send to dev1, the main thread used by
> >>> dev2
> >>> is calling the send(), while it is possible that dev2's pmd thread is
> >>> also calling
> >>> send(). So need a lock here.
> >>
> >> But they will send to different tx queues.
> >
> > OK, I think you are talking about the XPS feature (dynamic txqs).
> > I thought XPS can only work when all between pmd threads.
> > So adding a lock here.
> > The patch currently doesn't work without the lock. Let me investigate
> > more.
> >
> More details.
> On my test using veth (only have 1 rxq 1 txq)
> Somehow the main thread is assigned to use queue id = 2, which causes
> segfault.
> >>>
> >>> That is very strange...
> >>> Could you, please, share your test setup? I'll try to reproduce.
> >>>
> >>> Best regards, Ilya Maximets.
> >>
> >> Sorry, somehow I miss your reply in my mailbox.
> >> I simply tested it using two namespaces, one device as afxdp, the
> >> other as afxdp-nonpmd.
> >> ---
> >> # start ovs-vswitchd and ovsdb-server
> >>
> >> ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
> >> ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xf
> >>
> >> ip netns add at_ns0
> >> ip link add p0 type veth peer name afxdp-p0
> >> ip link set p0
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On 3/6/20 5:11 PM, Ilya Maximets wrote:
> On 3/4/20 7:09 PM, William Tu wrote:
>> On Fri, Feb 28, 2020 at 12:12 AM Ilya Maximets wrote:
>>>
>>> On 2/27/20 8:52 PM, William Tu wrote:
On Thu, Feb 27, 2020 at 11:10 AM William Tu wrote:
>
> On Thu, Feb 27, 2020 at 9:54 AM Ilya Maximets wrote:
>>
>> On 2/27/20 6:51 PM, William Tu wrote:
>>> On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets
>>> wrote:
On 2/27/20 6:30 PM, William Tu wrote:
> The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
> interrupt mode. This is similar to 'type=afxdp', except that the
> is_pmd field is set to false. As a result, the packet processing
> is handled by main thread, not pmd thread. This avoids burning
> the CPU to always 100% when there is no traffic.
>
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
> Signed-off-by: William Tu
> ---
> NEWS | 3 +++
> lib/netdev-afxdp.c| 20 +++-
> lib/netdev-linux.c| 20
> lib/netdev-provider.h | 1 +
> lib/netdev.c | 1 +
> tests/system-afxdp.at | 23 +++
> 6 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index f62ef1f47ea8..594c55dc11d6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,6 +4,9 @@ Post-v2.13.0
> - OpenFlow:
> * The OpenFlow ofp_desc/serial_num may now be configured by
> setting the
> value of other-config:dp-sn in the Bridge table.
> + - AF_XDP:
> + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU
> cycles
> + by enabling interrupt mode.
>
>
> v2.13.0 - 14 Feb 2020
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index 482400d8d135..cd2c7c381139 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
> );
> };
>
> +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode.
> */
> +static bool
> +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
> +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
> +}
> +
> #ifdef HAVE_XDP_NEED_WAKEUP
> static inline void
> xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev,
> int qid,
> struct netdev_linux *dev;
> int ret;
>
> -if (concurrent_txq) {
> +/* Lock is required when mixing afxdp pmd and nonpmd mode.
> + * ex: one device is created 'afxdp', the other is
> 'afxdp-nonpmd'.
> + */
> +if (concurrent_txq || (nonpmd_cnt != 0)) {
I'm confused about this change. Are you expecting same device being
opened twice with different netdev type? Database should not allow
this.
>>>
>>> Not the same device.
>>>
>>> For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
>>> When dev2 receives a packet and send to dev1, the main thread used by
>>> dev2
>>> is calling the send(), while it is possible that dev2's pmd thread is
>>> also calling
>>> send(). So need a lock here.
>>
>> But they will send to different tx queues.
>
> OK, I think you are talking about the XPS feature (dynamic txqs).
> I thought XPS can only work when all between pmd threads.
> So adding a lock here.
> The patch currently doesn't work without the lock. Let me investigate
> more.
>
More details.
On my test using veth (only have 1 rxq 1 txq)
Somehow the main thread is assigned to use queue id = 2, which causes
segfault.
>>>
>>> That is very strange...
>>> Could you, please, share your test setup? I'll try to reproduce.
>>>
>>> Best regards, Ilya Maximets.
>>
>> Sorry, somehow I miss your reply in my mailbox.
>> I simply tested it using two namespaces, one device as afxdp, the
>> other as afxdp-nonpmd.
>> ---
>> # start ovs-vswitchd and ovsdb-server
>>
>> ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
>> ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xf
>>
>> ip netns add at_ns0
>> ip link add p0 type veth peer name afxdp-p0
>> ip link set p0 netns at_ns0
>> ip link set dev afxdp-p0 up
>> ovs-vsctl add-port br0 afxdp-p0
>> ovs-vsctl -- set interface afxdp-p0 options:n_rxq=1
>> type="afxdp-nonpmd" options:xdp-mode=generic
>>
>> ip netns exec at_ns0 sh << NS_EXEC_HEREDOC
>> ip addr add "10.1.1.1/24" dev p0
>> ip link set dev p0 up
>> NS_EXEC_HEREDOC
>>
>> ip netns add at_ns1
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On 3/4/20 7:09 PM, William Tu wrote:
> On Fri, Feb 28, 2020 at 12:12 AM Ilya Maximets wrote:
>>
>> On 2/27/20 8:52 PM, William Tu wrote:
>>> On Thu, Feb 27, 2020 at 11:10 AM William Tu wrote:
On Thu, Feb 27, 2020 at 9:54 AM Ilya Maximets wrote:
>
> On 2/27/20 6:51 PM, William Tu wrote:
>> On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets wrote:
>>>
>>> On 2/27/20 6:30 PM, William Tu wrote:
The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
interrupt mode. This is similar to 'type=afxdp', except that the
is_pmd field is set to false. As a result, the packet processing
is handled by main thread, not pmd thread. This avoids burning
the CPU to always 100% when there is no traffic.
Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
Signed-off-by: William Tu
---
NEWS | 3 +++
lib/netdev-afxdp.c| 20 +++-
lib/netdev-linux.c| 20
lib/netdev-provider.h | 1 +
lib/netdev.c | 1 +
tests/system-afxdp.at | 23 +++
6 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index f62ef1f47ea8..594c55dc11d6 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ Post-v2.13.0
- OpenFlow:
* The OpenFlow ofp_desc/serial_num may now be configured by
setting the
value of other-config:dp-sn in the Bridge table.
+ - AF_XDP:
+ * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU
cycles
+ by enabling interrupt mode.
v2.13.0 - 14 Feb 2020
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index 482400d8d135..cd2c7c381139 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
);
};
+static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode. */
+static bool
+netdev_is_afxdp_nonpmd(struct netdev *netdev) {
+return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
+}
+
#ifdef HAVE_XDP_NEED_WAKEUP
static inline void
xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
@@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev,
int qid,
struct netdev_linux *dev;
int ret;
-if (concurrent_txq) {
+/* Lock is required when mixing afxdp pmd and nonpmd mode.
+ * ex: one device is created 'afxdp', the other is 'afxdp-nonpmd'.
+ */
+if (concurrent_txq || (nonpmd_cnt != 0)) {
>>>
>>> I'm confused about this change. Are you expecting same device being
>>> opened twice with different netdev type? Database should not allow
>>> this.
>>
>> Not the same device.
>>
>> For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
>> When dev2 receives a packet and send to dev1, the main thread used by
>> dev2
>> is calling the send(), while it is possible that dev2's pmd thread is
>> also calling
>> send(). So need a lock here.
>
> But they will send to different tx queues.
OK, I think you are talking about the XPS feature (dynamic txqs).
I thought XPS can only work when all between pmd threads.
So adding a lock here.
The patch currently doesn't work without the lock. Let me investigate more.
>>> More details.
>>> On my test using veth (only have 1 rxq 1 txq)
>>> Somehow the main thread is assigned to use queue id = 2, which causes
>>> segfault.
>>
>> That is very strange...
>> Could you, please, share your test setup? I'll try to reproduce.
>>
>> Best regards, Ilya Maximets.
>
> Sorry, somehow I miss your reply in my mailbox.
> I simply tested it using two namespaces, one device as afxdp, the
> other as afxdp-nonpmd.
> ---
> # start ovs-vswitchd and ovsdb-server
>
> ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
> ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xf
>
> ip netns add at_ns0
> ip link add p0 type veth peer name afxdp-p0
> ip link set p0 netns at_ns0
> ip link set dev afxdp-p0 up
> ovs-vsctl add-port br0 afxdp-p0
> ovs-vsctl -- set interface afxdp-p0 options:n_rxq=1
> type="afxdp-nonpmd" options:xdp-mode=generic
>
> ip netns exec at_ns0 sh << NS_EXEC_HEREDOC
> ip addr add "10.1.1.1/24" dev p0
> ip link set dev p0 up
> NS_EXEC_HEREDOC
>
> ip netns add at_ns1
> ip link add p1 type veth peer name afxdp-p1
> ip link set p1 netns at_ns1
> ip link set dev afxdp-p1 up
> ovs-vsctl add-port br0 afxdp-p1 -- \
>set interface afxdp-p1 type="afxdp"
>
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On Fri, Feb 28, 2020 at 12:12 AM Ilya Maximets wrote:
>
> On 2/27/20 8:52 PM, William Tu wrote:
> > On Thu, Feb 27, 2020 at 11:10 AM William Tu wrote:
> >>
> >> On Thu, Feb 27, 2020 at 9:54 AM Ilya Maximets wrote:
> >>>
> >>> On 2/27/20 6:51 PM, William Tu wrote:
> On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets wrote:
> >
> > On 2/27/20 6:30 PM, William Tu wrote:
> >> The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
> >> interrupt mode. This is similar to 'type=afxdp', except that the
> >> is_pmd field is set to false. As a result, the packet processing
> >> is handled by main thread, not pmd thread. This avoids burning
> >> the CPU to always 100% when there is no traffic.
> >>
> >> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
> >> Signed-off-by: William Tu
> >> ---
> >> NEWS | 3 +++
> >> lib/netdev-afxdp.c| 20 +++-
> >> lib/netdev-linux.c| 20
> >> lib/netdev-provider.h | 1 +
> >> lib/netdev.c | 1 +
> >> tests/system-afxdp.at | 23 +++
> >> 6 files changed, 67 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/NEWS b/NEWS
> >> index f62ef1f47ea8..594c55dc11d6 100644
> >> --- a/NEWS
> >> +++ b/NEWS
> >> @@ -4,6 +4,9 @@ Post-v2.13.0
> >> - OpenFlow:
> >> * The OpenFlow ofp_desc/serial_num may now be configured by
> >> setting the
> >> value of other-config:dp-sn in the Bridge table.
> >> + - AF_XDP:
> >> + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU
> >> cycles
> >> + by enabling interrupt mode.
> >>
> >>
> >> v2.13.0 - 14 Feb 2020
> >> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> >> index 482400d8d135..cd2c7c381139 100644
> >> --- a/lib/netdev-afxdp.c
> >> +++ b/lib/netdev-afxdp.c
> >> @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
> >> );
> >> };
> >>
> >> +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode. */
> >> +static bool
> >> +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
> >> +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
> >> +}
> >> +
> >> #ifdef HAVE_XDP_NEED_WAKEUP
> >> static inline void
> >> xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> >> @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev,
> >> int qid,
> >> struct netdev_linux *dev;
> >> int ret;
> >>
> >> -if (concurrent_txq) {
> >> +/* Lock is required when mixing afxdp pmd and nonpmd mode.
> >> + * ex: one device is created 'afxdp', the other is 'afxdp-nonpmd'.
> >> + */
> >> +if (concurrent_txq || (nonpmd_cnt != 0)) {
> >
> > I'm confused about this change. Are you expecting same device being
> > opened twice with different netdev type? Database should not allow
> > this.
>
> Not the same device.
>
> For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
> When dev2 receives a packet and send to dev1, the main thread used by
> dev2
> is calling the send(), while it is possible that dev2's pmd thread is
> also calling
> send(). So need a lock here.
> >>>
> >>> But they will send to different tx queues.
> >>
> >> OK, I think you are talking about the XPS feature (dynamic txqs).
> >> I thought XPS can only work when all between pmd threads.
> >> So adding a lock here.
> >> The patch currently doesn't work without the lock. Let me investigate more.
> >>
> > More details.
> > On my test using veth (only have 1 rxq 1 txq)
> > Somehow the main thread is assigned to use queue id = 2, which causes
> > segfault.
>
> That is very strange...
> Could you, please, share your test setup? I'll try to reproduce.
>
> Best regards, Ilya Maximets.
Sorry, somehow I miss your reply in my mailbox.
I simply tested it using two namespaces, one device as afxdp, the
other as afxdp-nonpmd.
---
# start ovs-vswitchd and ovsdb-server
ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xf
ip netns add at_ns0
ip link add p0 type veth peer name afxdp-p0
ip link set p0 netns at_ns0
ip link set dev afxdp-p0 up
ovs-vsctl add-port br0 afxdp-p0
ovs-vsctl -- set interface afxdp-p0 options:n_rxq=1
type="afxdp-nonpmd" options:xdp-mode=generic
ip netns exec at_ns0 sh << NS_EXEC_HEREDOC
ip addr add "10.1.1.1/24" dev p0
ip link set dev p0 up
NS_EXEC_HEREDOC
ip netns add at_ns1
ip link add p1 type veth peer name afxdp-p1
ip link set p1 netns at_ns1
ip link set dev afxdp-p1 up
ovs-vsctl add-port br0 afxdp-p1 -- \
set interface afxdp-p1 type="afxdp"
options:xdp-mode=generic options:n_rxq=1
ip netns exec at_ns1 sh << NS_EXEC_HEREDOC
ip addr add "10.1.1.
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On 2/27/20 8:52 PM, William Tu wrote:
> On Thu, Feb 27, 2020 at 11:10 AM William Tu wrote:
>>
>> On Thu, Feb 27, 2020 at 9:54 AM Ilya Maximets wrote:
>>>
>>> On 2/27/20 6:51 PM, William Tu wrote:
On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets wrote:
>
> On 2/27/20 6:30 PM, William Tu wrote:
>> The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
>> interrupt mode. This is similar to 'type=afxdp', except that the
>> is_pmd field is set to false. As a result, the packet processing
>> is handled by main thread, not pmd thread. This avoids burning
>> the CPU to always 100% when there is no traffic.
>>
>> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
>> Signed-off-by: William Tu
>> ---
>> NEWS | 3 +++
>> lib/netdev-afxdp.c| 20 +++-
>> lib/netdev-linux.c| 20
>> lib/netdev-provider.h | 1 +
>> lib/netdev.c | 1 +
>> tests/system-afxdp.at | 23 +++
>> 6 files changed, 67 insertions(+), 1 deletion(-)
>>
>> diff --git a/NEWS b/NEWS
>> index f62ef1f47ea8..594c55dc11d6 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -4,6 +4,9 @@ Post-v2.13.0
>> - OpenFlow:
>> * The OpenFlow ofp_desc/serial_num may now be configured by
>> setting the
>> value of other-config:dp-sn in the Bridge table.
>> + - AF_XDP:
>> + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU
>> cycles
>> + by enabling interrupt mode.
>>
>>
>> v2.13.0 - 14 Feb 2020
>> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
>> index 482400d8d135..cd2c7c381139 100644
>> --- a/lib/netdev-afxdp.c
>> +++ b/lib/netdev-afxdp.c
>> @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
>> );
>> };
>>
>> +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode. */
>> +static bool
>> +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
>> +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
>> +}
>> +
>> #ifdef HAVE_XDP_NEED_WAKEUP
>> static inline void
>> xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
>> @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev,
>> int qid,
>> struct netdev_linux *dev;
>> int ret;
>>
>> -if (concurrent_txq) {
>> +/* Lock is required when mixing afxdp pmd and nonpmd mode.
>> + * ex: one device is created 'afxdp', the other is 'afxdp-nonpmd'.
>> + */
>> +if (concurrent_txq || (nonpmd_cnt != 0)) {
>
> I'm confused about this change. Are you expecting same device being
> opened twice with different netdev type? Database should not allow this.
Not the same device.
For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
When dev2 receives a packet and send to dev1, the main thread used by dev2
is calling the send(), while it is possible that dev2's pmd thread is
also calling
send(). So need a lock here.
>>>
>>> But they will send to different tx queues.
>>
>> OK, I think you are talking about the XPS feature (dynamic txqs).
>> I thought XPS can only work when all between pmd threads.
>> So adding a lock here.
>> The patch currently doesn't work without the lock. Let me investigate more.
>>
> More details.
> On my test using veth (only have 1 rxq 1 txq)
> Somehow the main thread is assigned to use queue id = 2, which causes
> segfault.
That is very strange...
Could you, please, share your test setup? I'll try to reproduce.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On Thu, Feb 27, 2020 at 11:10 AM William Tu wrote:
>
> On Thu, Feb 27, 2020 at 9:54 AM Ilya Maximets wrote:
> >
> > On 2/27/20 6:51 PM, William Tu wrote:
> > > On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets wrote:
> > >>
> > >> On 2/27/20 6:30 PM, William Tu wrote:
> > >>> The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
> > >>> interrupt mode. This is similar to 'type=afxdp', except that the
> > >>> is_pmd field is set to false. As a result, the packet processing
> > >>> is handled by main thread, not pmd thread. This avoids burning
> > >>> the CPU to always 100% when there is no traffic.
> > >>>
> > >>> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
> > >>> Signed-off-by: William Tu
> > >>> ---
> > >>> NEWS | 3 +++
> > >>> lib/netdev-afxdp.c| 20 +++-
> > >>> lib/netdev-linux.c| 20
> > >>> lib/netdev-provider.h | 1 +
> > >>> lib/netdev.c | 1 +
> > >>> tests/system-afxdp.at | 23 +++
> > >>> 6 files changed, 67 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/NEWS b/NEWS
> > >>> index f62ef1f47ea8..594c55dc11d6 100644
> > >>> --- a/NEWS
> > >>> +++ b/NEWS
> > >>> @@ -4,6 +4,9 @@ Post-v2.13.0
> > >>> - OpenFlow:
> > >>> * The OpenFlow ofp_desc/serial_num may now be configured by
> > >>> setting the
> > >>> value of other-config:dp-sn in the Bridge table.
> > >>> + - AF_XDP:
> > >>> + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU
> > >>> cycles
> > >>> + by enabling interrupt mode.
> > >>>
> > >>>
> > >>> v2.13.0 - 14 Feb 2020
> > >>> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> > >>> index 482400d8d135..cd2c7c381139 100644
> > >>> --- a/lib/netdev-afxdp.c
> > >>> +++ b/lib/netdev-afxdp.c
> > >>> @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
> > >>> );
> > >>> };
> > >>>
> > >>> +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode. */
> > >>> +static bool
> > >>> +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
> > >>> +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
> > >>> +}
> > >>> +
> > >>> #ifdef HAVE_XDP_NEED_WAKEUP
> > >>> static inline void
> > >>> xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> > >>> @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev,
> > >>> int qid,
> > >>> struct netdev_linux *dev;
> > >>> int ret;
> > >>>
> > >>> -if (concurrent_txq) {
> > >>> +/* Lock is required when mixing afxdp pmd and nonpmd mode.
> > >>> + * ex: one device is created 'afxdp', the other is 'afxdp-nonpmd'.
> > >>> + */
> > >>> +if (concurrent_txq || (nonpmd_cnt != 0)) {
> > >>
> > >> I'm confused about this change. Are you expecting same device being
> > >> opened twice with different netdev type? Database should not allow this.
> > >
> > > Not the same device.
> > >
> > > For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
> > > When dev2 receives a packet and send to dev1, the main thread used by dev2
> > > is calling the send(), while it is possible that dev2's pmd thread is
> > > also calling
> > > send(). So need a lock here.
> >
> > But they will send to different tx queues.
>
> OK, I think you are talking about the XPS feature (dynamic txqs).
> I thought XPS can only work when all between pmd threads.
> So adding a lock here.
> The patch currently doesn't work without the lock. Let me investigate more.
>
More details.
On my test using veth (only have 1 rxq 1 txq)
Somehow the main thread is assigned to use queue id = 2, which causes segfault.
I will send v3 patch.
Thanks
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On Thu, Feb 27, 2020 at 9:54 AM Ilya Maximets wrote:
>
> On 2/27/20 6:51 PM, William Tu wrote:
> > On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets wrote:
> >>
> >> On 2/27/20 6:30 PM, William Tu wrote:
> >>> The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
> >>> interrupt mode. This is similar to 'type=afxdp', except that the
> >>> is_pmd field is set to false. As a result, the packet processing
> >>> is handled by main thread, not pmd thread. This avoids burning
> >>> the CPU to always 100% when there is no traffic.
> >>>
> >>> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
> >>> Signed-off-by: William Tu
> >>> ---
> >>> NEWS | 3 +++
> >>> lib/netdev-afxdp.c| 20 +++-
> >>> lib/netdev-linux.c| 20
> >>> lib/netdev-provider.h | 1 +
> >>> lib/netdev.c | 1 +
> >>> tests/system-afxdp.at | 23 +++
> >>> 6 files changed, 67 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/NEWS b/NEWS
> >>> index f62ef1f47ea8..594c55dc11d6 100644
> >>> --- a/NEWS
> >>> +++ b/NEWS
> >>> @@ -4,6 +4,9 @@ Post-v2.13.0
> >>> - OpenFlow:
> >>> * The OpenFlow ofp_desc/serial_num may now be configured by setting
> >>> the
> >>> value of other-config:dp-sn in the Bridge table.
> >>> + - AF_XDP:
> >>> + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU
> >>> cycles
> >>> + by enabling interrupt mode.
> >>>
> >>>
> >>> v2.13.0 - 14 Feb 2020
> >>> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> >>> index 482400d8d135..cd2c7c381139 100644
> >>> --- a/lib/netdev-afxdp.c
> >>> +++ b/lib/netdev-afxdp.c
> >>> @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
> >>> );
> >>> };
> >>>
> >>> +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode. */
> >>> +static bool
> >>> +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
> >>> +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
> >>> +}
> >>> +
> >>> #ifdef HAVE_XDP_NEED_WAKEUP
> >>> static inline void
> >>> xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> >>> @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev, int
> >>> qid,
> >>> struct netdev_linux *dev;
> >>> int ret;
> >>>
> >>> -if (concurrent_txq) {
> >>> +/* Lock is required when mixing afxdp pmd and nonpmd mode.
> >>> + * ex: one device is created 'afxdp', the other is 'afxdp-nonpmd'.
> >>> + */
> >>> +if (concurrent_txq || (nonpmd_cnt != 0)) {
> >>
> >> I'm confused about this change. Are you expecting same device being
> >> opened twice with different netdev type? Database should not allow this.
> >
> > Not the same device.
> >
> > For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
> > When dev2 receives a packet and send to dev1, the main thread used by dev2
> > is calling the send(), while it is possible that dev2's pmd thread is
> > also calling
> > send(). So need a lock here.
>
> But they will send to different tx queues.
OK, I think you are talking about the XPS feature (dynamic txqs).
I thought XPS can only work when all between pmd threads.
So adding a lock here.
The patch currently doesn't work without the lock. Let me investigate more.
Thanks
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On 2/27/20 6:51 PM, William Tu wrote:
> On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets wrote:
>>
>> On 2/27/20 6:30 PM, William Tu wrote:
>>> The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
>>> interrupt mode. This is similar to 'type=afxdp', except that the
>>> is_pmd field is set to false. As a result, the packet processing
>>> is handled by main thread, not pmd thread. This avoids burning
>>> the CPU to always 100% when there is no traffic.
>>>
>>> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
>>> Signed-off-by: William Tu
>>> ---
>>> NEWS | 3 +++
>>> lib/netdev-afxdp.c| 20 +++-
>>> lib/netdev-linux.c| 20
>>> lib/netdev-provider.h | 1 +
>>> lib/netdev.c | 1 +
>>> tests/system-afxdp.at | 23 +++
>>> 6 files changed, 67 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/NEWS b/NEWS
>>> index f62ef1f47ea8..594c55dc11d6 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -4,6 +4,9 @@ Post-v2.13.0
>>> - OpenFlow:
>>> * The OpenFlow ofp_desc/serial_num may now be configured by setting
>>> the
>>> value of other-config:dp-sn in the Bridge table.
>>> + - AF_XDP:
>>> + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU cycles
>>> + by enabling interrupt mode.
>>>
>>>
>>> v2.13.0 - 14 Feb 2020
>>> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
>>> index 482400d8d135..cd2c7c381139 100644
>>> --- a/lib/netdev-afxdp.c
>>> +++ b/lib/netdev-afxdp.c
>>> @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
>>> );
>>> };
>>>
>>> +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode. */
>>> +static bool
>>> +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
>>> +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
>>> +}
>>> +
>>> #ifdef HAVE_XDP_NEED_WAKEUP
>>> static inline void
>>> xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
>>> @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev, int
>>> qid,
>>> struct netdev_linux *dev;
>>> int ret;
>>>
>>> -if (concurrent_txq) {
>>> +/* Lock is required when mixing afxdp pmd and nonpmd mode.
>>> + * ex: one device is created 'afxdp', the other is 'afxdp-nonpmd'.
>>> + */
>>> +if (concurrent_txq || (nonpmd_cnt != 0)) {
>>
>> I'm confused about this change. Are you expecting same device being
>> opened twice with different netdev type? Database should not allow this.
>
> Not the same device.
>
> For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
> When dev2 receives a packet and send to dev1, the main thread used by dev2
> is calling the send(), while it is possible that dev2's pmd thread is
> also calling
> send(). So need a lock here.
But they will send to different tx queues.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets wrote:
>
> On 2/27/20 6:30 PM, William Tu wrote:
> > The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
> > interrupt mode. This is similar to 'type=afxdp', except that the
> > is_pmd field is set to false. As a result, the packet processing
> > is handled by main thread, not pmd thread. This avoids burning
> > the CPU to always 100% when there is no traffic.
> >
> > Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
> > Signed-off-by: William Tu
> > ---
> > NEWS | 3 +++
> > lib/netdev-afxdp.c| 20 +++-
> > lib/netdev-linux.c| 20
> > lib/netdev-provider.h | 1 +
> > lib/netdev.c | 1 +
> > tests/system-afxdp.at | 23 +++
> > 6 files changed, 67 insertions(+), 1 deletion(-)
> >
> > diff --git a/NEWS b/NEWS
> > index f62ef1f47ea8..594c55dc11d6 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -4,6 +4,9 @@ Post-v2.13.0
> > - OpenFlow:
> > * The OpenFlow ofp_desc/serial_num may now be configured by setting
> > the
> > value of other-config:dp-sn in the Bridge table.
> > + - AF_XDP:
> > + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU cycles
> > + by enabling interrupt mode.
> >
> >
> > v2.13.0 - 14 Feb 2020
> > diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> > index 482400d8d135..cd2c7c381139 100644
> > --- a/lib/netdev-afxdp.c
> > +++ b/lib/netdev-afxdp.c
> > @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
> > );
> > };
> >
> > +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode. */
> > +static bool
> > +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
> > +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
> > +}
> > +
> > #ifdef HAVE_XDP_NEED_WAKEUP
> > static inline void
> > xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> > @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev, int
> > qid,
> > struct netdev_linux *dev;
> > int ret;
> >
> > -if (concurrent_txq) {
> > +/* Lock is required when mixing afxdp pmd and nonpmd mode.
> > + * ex: one device is created 'afxdp', the other is 'afxdp-nonpmd'.
> > + */
> > +if (concurrent_txq || (nonpmd_cnt != 0)) {
>
> I'm confused about this change. Are you expecting same device being
> opened twice with different netdev type? Database should not allow this.
Not the same device.
For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
When dev2 receives a packet and send to dev1, the main thread used by dev2
is calling the send(), while it is possible that dev2's pmd thread is
also calling
send(). So need a lock here.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On 2/27/20 6:30 PM, William Tu wrote:
> The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
> interrupt mode. This is similar to 'type=afxdp', except that the
> is_pmd field is set to false. As a result, the packet processing
> is handled by main thread, not pmd thread. This avoids burning
> the CPU to always 100% when there is no traffic.
>
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
> Signed-off-by: William Tu
> ---
> NEWS | 3 +++
> lib/netdev-afxdp.c| 20 +++-
> lib/netdev-linux.c| 20
> lib/netdev-provider.h | 1 +
> lib/netdev.c | 1 +
> tests/system-afxdp.at | 23 +++
> 6 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index f62ef1f47ea8..594c55dc11d6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,6 +4,9 @@ Post-v2.13.0
> - OpenFlow:
> * The OpenFlow ofp_desc/serial_num may now be configured by setting the
> value of other-config:dp-sn in the Bridge table.
> + - AF_XDP:
> + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU cycles
> + by enabling interrupt mode.
>
>
> v2.13.0 - 14 Feb 2020
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index 482400d8d135..cd2c7c381139 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
> );
> };
>
> +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode. */
> +static bool
> +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
> +return netdev_get_class(netdev) == &netdev_afxdp_nonpmd_class;
> +}
> +
> #ifdef HAVE_XDP_NEED_WAKEUP
> static inline void
> xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev, int qid,
> struct netdev_linux *dev;
> int ret;
>
> -if (concurrent_txq) {
> +/* Lock is required when mixing afxdp pmd and nonpmd mode.
> + * ex: one device is created 'afxdp', the other is 'afxdp-nonpmd'.
> + */
> +if (concurrent_txq || (nonpmd_cnt != 0)) {
I'm confused about this change. Are you expecting same device being
opened twice with different netdev type? Database should not allow this.
> dev = netdev_linux_cast(netdev);
> qid = qid % netdev_n_txq(netdev);
>
> @@ -1159,6 +1168,7 @@ libbpf_print(enum libbpf_print_level level,
> int netdev_afxdp_init(void)
> {
> libbpf_set_print(libbpf_print);
> +nonpmd_cnt = 0;
> return 0;
> }
>
> @@ -1188,6 +1198,10 @@ netdev_afxdp_construct(struct netdev *netdev)
> dev->tx_locks = NULL;
>
> netdev_request_reconfigure(netdev);
> +
> +if (netdev_is_afxdp_nonpmd(netdev)) {
> +nonpmd_cnt++;
> +}
> return 0;
> }
>
> @@ -1208,6 +1222,10 @@ netdev_afxdp_destruct(struct netdev *netdev)
>
> xsk_destroy_all(netdev);
> ovs_mutex_destroy(&dev->mutex);
> +
> +if (netdev_is_afxdp_nonpmd(netdev)) {
> +nonpmd_cnt--;
> +}
> }
>
> int
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index c6f3d27409b6..75b3b55d3cad 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -3605,6 +3605,26 @@ const struct netdev_class netdev_afxdp_class = {
> .rxq_destruct = netdev_afxdp_rxq_destruct,
> .rxq_recv = netdev_afxdp_rxq_recv,
> };
> +
> +const struct netdev_class netdev_afxdp_nonpmd_class = {
> +NETDEV_LINUX_CLASS_COMMON,
> +.type = "afxdp-nonpmd",
> +.is_pmd = false,
> +.init = netdev_afxdp_init,
> +.construct = netdev_afxdp_construct,
> +.destruct = netdev_afxdp_destruct,
> +.get_stats = netdev_afxdp_get_stats,
> +.get_custom_stats = netdev_afxdp_get_custom_stats,
> +.get_status = netdev_linux_get_status,
> +.set_config = netdev_afxdp_set_config,
> +.get_config = netdev_afxdp_get_config,
> +.reconfigure = netdev_afxdp_reconfigure,
> +.get_numa_id = netdev_linux_get_numa_id,
> +.send = netdev_afxdp_batch_send,
> +.rxq_construct = netdev_afxdp_rxq_construct,
> +.rxq_destruct = netdev_afxdp_rxq_destruct,
> +.rxq_recv = netdev_afxdp_rxq_recv,
> +};
> #endif
>
>
> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
> index 22f4cde3337a..06c1d98a0b07 100644
> --- a/lib/netdev-provider.h
> +++ b/lib/netdev-provider.h
> @@ -848,6 +848,7 @@ extern const struct netdev_class netdev_tap_class;
>
> #ifdef HAVE_AF_XDP
> extern const struct netdev_class netdev_afxdp_class;
> +extern const struct netdev_class netdev_afxdp_nonpmd_class;
> #endif
> #ifdef __cplusplus
> }
> diff --git a/lib/netdev.c b/lib/netdev.c
> index f95b19af4da0..6d9723ebc3c1 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -154,6 +154,7 @@ netdev_initialize(void)
> netdev_register_flow_api_provider(&netdev_offload_tc);
> #ifdef HAVE_AF_XDP
> netdev_register_provider(&netdev_afxdp_class);
> +netdev_register_provider(&netdev_afxdp_non
