Re: [ovs-dev] 答复: 答复: [PATCH V3 2/4] Add GSO support for DPDK data path
On 2/8/21 8:54 PM, William Tu wrote:
> On Mon, Feb 8, 2021 at 6:36 AM Ilya Maximets wrote:
>>
>> On 2/8/21 1:56 AM, Yi Yang (杨燚)-云服务集团 wrote:
>>> Yes, GSO is ok, but GRO may be have that issue, I didn't see that issue in
>>> my openstack environment, so maybe it will be great if we can have a test
>>> case to trigger that issue.
>>>
>>> -邮件原件-
>>> 发件人: William Tu [mailto:u9012...@gmail.com]
>>> 发送时间: 2021年2月7日 23:46
>>> 收件人: Yi Yang (杨燚)-云服务集团
>>> 抄送: i.maxim...@ovn.org; yang_y...@163.com; ovs-dev@openvswitch.org;
>>> f...@sysclose.org
>>> 主题: Re: [ovs-dev] 答复: [PATCH V3 2/4] Add GSO support for DPDK data path
>>>
>>> On Tue, Oct 27, 2020 at 6:02 PM Yi Yang (杨燚)-云服务集团
>>> wrote:
>>>>
>>>> -邮件原件-
>>>> 发件人: dev [mailto:ovs-dev-boun...@openvswitch.org] 代表 Ilya Maximets
>>>> 发送时间: 2020年10月27日 21:03
>>>> 收件人: yang_y...@163.com; ovs-dev@openvswitch.org
>>>> 抄送: f...@sysclose.org; i.maxim...@ovn.org
>>>> 主题: Re: [ovs-dev] [PATCH V3 2/4] Add GSO support for DPDK data path
>>>>
>>>> On 8/7/20 12:56 PM, yang_y...@163.com wrote:
>>>>> From: Yi Yang
>>>>>
>>>>> GSO(Generic Segment Offload) can segment large UDP and TCP packet
>>>>> to small packets per MTU of destination , especially for the case
>>>>> that physical NIC can't do hardware offload VXLAN TSO and VXLAN UFO,
>>>>> GSO can make sure userspace TSO can still work but not drop.
>>>>>
>>>>> In addition, GSO can help improve UDP performane when UFO is enabled
>>>>> in VM.
>>>>>
>>>>> GSO can support TCP, UDP, VXLAN TCP, VXLAN UDP, it is done in Tx
>>>>> function of physical NIC.
>>>>>
>>>>> Signed-off-by: Yi Yang
>>>>> ---
>>>>> lib/dp-packet.h| 21 +++-
>>>>> lib/netdev-dpdk.c | 358
>>>>> +
>>>>> lib/netdev-linux.c | 17 ++-
>>>>> lib/netdev.c | 67 +++---
>>>>> 4 files changed, 417 insertions(+), 46 deletions(-)
>>>
>>> snip
>>>
>>>>>
>>>>> @@ -2339,24 +2428,19 @@ netdev_dpdk_prep_hwol_batch(struct netdev_dpdk
>>>>> *dev, struct rte_mbuf **pkts,
>>>>> return cnt;
>>>>> }
>>>>>
>>>>> -/* Tries to transmit 'pkts' to txq 'qid' of device 'dev'. Takes
>>>>> ownership of
>>>>> - * 'pkts', even in case of failure.
>>>>> - *
>>>>> - * Returns the number of packets that weren't transmitted. */
>>>>> static inline int -netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int
>>>>> qid,
>>>>> - struct rte_mbuf **pkts, int cnt)
>>>>> +__netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int qid,
>>>>> + struct rte_mbuf **pkts, int cnt)
>>>>> {
>>>>> uint32_t nb_tx = 0;
>>>>> -uint16_t nb_tx_prep = cnt;
>>>>> +uint32_t nb_tx_prep;
>>>>>
>>>>> -if (userspace_tso_enabled()) {
>>>>> -nb_tx_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
>>>>> -if (nb_tx_prep != cnt) {
>>>>> -VLOG_WARN_RL(, "%s: Output batch contains invalid
>>>>> packets. "
>>>>> - "Only %u/%u are valid: %s", dev->up.name,
>>>>> nb_tx_prep,
>>>>> - cnt, rte_strerror(rte_errno));
>>>>> -}
>>>>> +nb_tx_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
>>>>> +if (nb_tx_prep != cnt) {
>>>>> +VLOG_WARN_RL(, "%s: Output batch contains invalid packets. "
>>>>> + "Only %u/%u are valid: %s",
>>>>> + dev->up.name, nb_tx_prep,
>>>>> + cnt, rte_strerror(rte_errno));
>>>>> }
>>>>>
>>>>> while (nb_tx != nb_tx_prep) {
>>>>> @@ -2384,6 +2468,200 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev,
>>>>> int qid,
>>>>> return cnt - nb_tx;
>>>>> }
>>>>>
>>>>> +s
Re: [ovs-dev] 答复: 答复: [PATCH V3 2/4] Add GSO support for DPDK data path
On Mon, Feb 8, 2021 at 6:36 AM Ilya Maximets wrote:
>
> On 2/8/21 1:56 AM, Yi Yang (杨燚)-云服务集团 wrote:
> > Yes, GSO is ok, but GRO may be have that issue, I didn't see that issue in
> > my openstack environment, so maybe it will be great if we can have a test
> > case to trigger that issue.
> >
> > -邮件原件-
> > 发件人: William Tu [mailto:u9012...@gmail.com]
> > 发送时间: 2021年2月7日 23:46
> > 收件人: Yi Yang (杨燚)-云服务集团
> > 抄送: i.maxim...@ovn.org; yang_y...@163.com; ovs-dev@openvswitch.org;
> > f...@sysclose.org
> > 主题: Re: [ovs-dev] 答复: [PATCH V3 2/4] Add GSO support for DPDK data path
> >
> > On Tue, Oct 27, 2020 at 6:02 PM Yi Yang (杨燚)-云服务集团
> > wrote:
> >>
> >> -邮件原件-
> >> 发件人: dev [mailto:ovs-dev-boun...@openvswitch.org] 代表 Ilya Maximets
> >> 发送时间: 2020年10月27日 21:03
> >> 收件人: yang_y...@163.com; ovs-dev@openvswitch.org
> >> 抄送: f...@sysclose.org; i.maxim...@ovn.org
> >> 主题: Re: [ovs-dev] [PATCH V3 2/4] Add GSO support for DPDK data path
> >>
> >> On 8/7/20 12:56 PM, yang_y...@163.com wrote:
> >>> From: Yi Yang
> >>>
> >>> GSO(Generic Segment Offload) can segment large UDP and TCP packet
> >>> to small packets per MTU of destination , especially for the case
> >>> that physical NIC can't do hardware offload VXLAN TSO and VXLAN UFO,
> >>> GSO can make sure userspace TSO can still work but not drop.
> >>>
> >>> In addition, GSO can help improve UDP performane when UFO is enabled
> >>> in VM.
> >>>
> >>> GSO can support TCP, UDP, VXLAN TCP, VXLAN UDP, it is done in Tx
> >>> function of physical NIC.
> >>>
> >>> Signed-off-by: Yi Yang
> >>> ---
> >>> lib/dp-packet.h| 21 +++-
> >>> lib/netdev-dpdk.c | 358
> >>> +
> >>> lib/netdev-linux.c | 17 ++-
> >>> lib/netdev.c | 67 +++---
> >>> 4 files changed, 417 insertions(+), 46 deletions(-)
> >
> > snip
> >
> >>>
> >>> @@ -2339,24 +2428,19 @@ netdev_dpdk_prep_hwol_batch(struct netdev_dpdk
> >>> *dev, struct rte_mbuf **pkts,
> >>> return cnt;
> >>> }
> >>>
> >>> -/* Tries to transmit 'pkts' to txq 'qid' of device 'dev'. Takes
> >>> ownership of
> >>> - * 'pkts', even in case of failure.
> >>> - *
> >>> - * Returns the number of packets that weren't transmitted. */
> >>> static inline int -netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int
> >>> qid,
> >>> - struct rte_mbuf **pkts, int cnt)
> >>> +__netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int qid,
> >>> + struct rte_mbuf **pkts, int cnt)
> >>> {
> >>> uint32_t nb_tx = 0;
> >>> -uint16_t nb_tx_prep = cnt;
> >>> +uint32_t nb_tx_prep;
> >>>
> >>> -if (userspace_tso_enabled()) {
> >>> -nb_tx_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
> >>> -if (nb_tx_prep != cnt) {
> >>> -VLOG_WARN_RL(, "%s: Output batch contains invalid
> >>> packets. "
> >>> - "Only %u/%u are valid: %s", dev->up.name,
> >>> nb_tx_prep,
> >>> - cnt, rte_strerror(rte_errno));
> >>> -}
> >>> +nb_tx_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
> >>> +if (nb_tx_prep != cnt) {
> >>> +VLOG_WARN_RL(, "%s: Output batch contains invalid packets. "
> >>> + "Only %u/%u are valid: %s",
> >>> + dev->up.name, nb_tx_prep,
> >>> + cnt, rte_strerror(rte_errno));
> >>> }
> >>>
> >>> while (nb_tx != nb_tx_prep) {
> >>> @@ -2384,6 +2468,200 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev,
> >>> int qid,
> >>> return cnt - nb_tx;
> >>> }
> >>>
> >>> +static inline void
> >>> +set_multiseg_udptcp_cksum(struct rte_mbuf *mbuf)
> >>
> >> I didn't review the patch, only had a quick glance, but this part bothers
> >> me. OVS doesn't support multi-segment mbufs, so it should not be possible
> >> for such mbufs being transmitted by O
Re: [ovs-dev] 答复: 答复: [PATCH V3 2/4] Add GSO support for DPDK data path
On 2/8/21 1:56 AM, Yi Yang (杨燚)-云服务集团 wrote:
> Yes, GSO is ok, but GRO may be have that issue, I didn't see that issue in
> my openstack environment, so maybe it will be great if we can have a test
> case to trigger that issue.
>
> -邮件原件-
> 发件人: William Tu [mailto:u9012...@gmail.com]
> 发送时间: 2021年2月7日 23:46
> 收件人: Yi Yang (杨燚)-云服务集团
> 抄送: i.maxim...@ovn.org; yang_y...@163.com; ovs-dev@openvswitch.org;
> f...@sysclose.org
> 主题: Re: [ovs-dev] 答复: [PATCH V3 2/4] Add GSO support for DPDK data path
>
> On Tue, Oct 27, 2020 at 6:02 PM Yi Yang (杨燚)-云服务集团
> wrote:
>>
>> -邮件原件-
>> 发件人: dev [mailto:ovs-dev-boun...@openvswitch.org] 代表 Ilya Maximets
>> 发送时间: 2020年10月27日 21:03
>> 收件人: yang_y...@163.com; ovs-dev@openvswitch.org
>> 抄送: f...@sysclose.org; i.maxim...@ovn.org
>> 主题: Re: [ovs-dev] [PATCH V3 2/4] Add GSO support for DPDK data path
>>
>> On 8/7/20 12:56 PM, yang_y...@163.com wrote:
>>> From: Yi Yang
>>>
>>> GSO(Generic Segment Offload) can segment large UDP and TCP packet
>>> to small packets per MTU of destination , especially for the case
>>> that physical NIC can't do hardware offload VXLAN TSO and VXLAN UFO,
>>> GSO can make sure userspace TSO can still work but not drop.
>>>
>>> In addition, GSO can help improve UDP performane when UFO is enabled
>>> in VM.
>>>
>>> GSO can support TCP, UDP, VXLAN TCP, VXLAN UDP, it is done in Tx
>>> function of physical NIC.
>>>
>>> Signed-off-by: Yi Yang
>>> ---
>>> lib/dp-packet.h| 21 +++-
>>> lib/netdev-dpdk.c | 358
>>> +
>>> lib/netdev-linux.c | 17 ++-
>>> lib/netdev.c | 67 +++---
>>> 4 files changed, 417 insertions(+), 46 deletions(-)
>
> snip
>
>>>
>>> @@ -2339,24 +2428,19 @@ netdev_dpdk_prep_hwol_batch(struct netdev_dpdk
>>> *dev, struct rte_mbuf **pkts,
>>> return cnt;
>>> }
>>>
>>> -/* Tries to transmit 'pkts' to txq 'qid' of device 'dev'. Takes
>>> ownership of
>>> - * 'pkts', even in case of failure.
>>> - *
>>> - * Returns the number of packets that weren't transmitted. */
>>> static inline int -netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int
>>> qid,
>>> - struct rte_mbuf **pkts, int cnt)
>>> +__netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int qid,
>>> + struct rte_mbuf **pkts, int cnt)
>>> {
>>> uint32_t nb_tx = 0;
>>> -uint16_t nb_tx_prep = cnt;
>>> +uint32_t nb_tx_prep;
>>>
>>> -if (userspace_tso_enabled()) {
>>> -nb_tx_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
>>> -if (nb_tx_prep != cnt) {
>>> -VLOG_WARN_RL(, "%s: Output batch contains invalid packets. "
>>> - "Only %u/%u are valid: %s", dev->up.name,
>>> nb_tx_prep,
>>> - cnt, rte_strerror(rte_errno));
>>> -}
>>> +nb_tx_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
>>> +if (nb_tx_prep != cnt) {
>>> +VLOG_WARN_RL(, "%s: Output batch contains invalid packets. "
>>> + "Only %u/%u are valid: %s",
>>> + dev->up.name, nb_tx_prep,
>>> + cnt, rte_strerror(rte_errno));
>>> }
>>>
>>> while (nb_tx != nb_tx_prep) {
>>> @@ -2384,6 +2468,200 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev,
>>> int qid,
>>> return cnt - nb_tx;
>>> }
>>>
>>> +static inline void
>>> +set_multiseg_udptcp_cksum(struct rte_mbuf *mbuf)
>>
>> I didn't review the patch, only had a quick glance, but this part bothers
>> me. OVS doesn't support multi-segment mbufs, so it should not be possible
>> for such mbufs being transmitted by OVS. So, I do not understand why this
>> function needs to work with such mbufs.
>>
>> [Yi Yang] Only DPDK driver/Tx function will use it, not OVS,
>> set_multiseg_udptcp_cksum is called in GSO part, it is last step before Tx
>> function, it is a big external mbuf before rte_gso_segment, that isn't a
>> multi-segmented mbuf.
>>
>
> Hi Ilya,
>
> Now I understand Yi Yang's point better and I agree with him.
> Looks like the patch does the GSO at the DPDK TX function.
> It creat
[ovs-dev] 答复: 答复: [PATCH V3 2/4] Add GSO support for DPDK data path
Yes, GSO is ok, but GRO may be have that issue, I didn't see that issue in my
openstack environment, so maybe it will be great if we can have a test case to
trigger that issue.
-邮件原件-
发件人: William Tu [mailto:u9012...@gmail.com]
发送时间: 2021年2月7日 23:46
收件人: Yi Yang (杨燚)-云服务集团
抄送: i.maxim...@ovn.org; yang_y...@163.com; ovs-dev@openvswitch.org;
f...@sysclose.org
主题: Re: [ovs-dev] 答复: [PATCH V3 2/4] Add GSO support for DPDK data path
On Tue, Oct 27, 2020 at 6:02 PM Yi Yang (杨燚)-云服务集团 wrote:
>
> -邮件原件-
> 发件人: dev [mailto:ovs-dev-boun...@openvswitch.org] 代表 Ilya Maximets
> 发送时间: 2020年10月27日 21:03
> 收件人: yang_y...@163.com; ovs-dev@openvswitch.org
> 抄送: f...@sysclose.org; i.maxim...@ovn.org
> 主题: Re: [ovs-dev] [PATCH V3 2/4] Add GSO support for DPDK data path
>
> On 8/7/20 12:56 PM, yang_y...@163.com wrote:
> > From: Yi Yang
> >
> > GSO(Generic Segment Offload) can segment large UDP and TCP packet
> > to small packets per MTU of destination , especially for the case
> > that physical NIC can't do hardware offload VXLAN TSO and VXLAN UFO,
> > GSO can make sure userspace TSO can still work but not drop.
> >
> > In addition, GSO can help improve UDP performane when UFO is enabled
> > in VM.
> >
> > GSO can support TCP, UDP, VXLAN TCP, VXLAN UDP, it is done in Tx
> > function of physical NIC.
> >
> > Signed-off-by: Yi Yang
> > ---
> > lib/dp-packet.h| 21 +++-
> > lib/netdev-dpdk.c | 358
> > +
> > lib/netdev-linux.c | 17 ++-
> > lib/netdev.c | 67 +++---
> > 4 files changed, 417 insertions(+), 46 deletions(-)
snip
> >
> > @@ -2339,24 +2428,19 @@ netdev_dpdk_prep_hwol_batch(struct netdev_dpdk
> > *dev, struct rte_mbuf **pkts,
> > return cnt;
> > }
> >
> > -/* Tries to transmit 'pkts' to txq 'qid' of device 'dev'. Takes
> > ownership of
> > - * 'pkts', even in case of failure.
> > - *
> > - * Returns the number of packets that weren't transmitted. */
> > static inline int -netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int
> > qid,
> > - struct rte_mbuf **pkts, int cnt)
> > +__netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int qid,
> > + struct rte_mbuf **pkts, int cnt)
> > {
> > uint32_t nb_tx = 0;
> > -uint16_t nb_tx_prep = cnt;
> > +uint32_t nb_tx_prep;
> >
> > -if (userspace_tso_enabled()) {
> > -nb_tx_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
> > -if (nb_tx_prep != cnt) {
> > -VLOG_WARN_RL(, "%s: Output batch contains invalid packets. "
> > - "Only %u/%u are valid: %s", dev->up.name,
> > nb_tx_prep,
> > - cnt, rte_strerror(rte_errno));
> > -}
> > +nb_tx_prep = rte_eth_tx_prepare(dev->port_id, qid, pkts, cnt);
> > +if (nb_tx_prep != cnt) {
> > +VLOG_WARN_RL(, "%s: Output batch contains invalid packets. "
> > + "Only %u/%u are valid: %s",
> > + dev->up.name, nb_tx_prep,
> > + cnt, rte_strerror(rte_errno));
> > }
> >
> > while (nb_tx != nb_tx_prep) {
> > @@ -2384,6 +2468,200 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev,
> > int qid,
> > return cnt - nb_tx;
> > }
> >
> > +static inline void
> > +set_multiseg_udptcp_cksum(struct rte_mbuf *mbuf)
>
> I didn't review the patch, only had a quick glance, but this part bothers me.
> OVS doesn't support multi-segment mbufs, so it should not be possible for
> such mbufs being transmitted by OVS. So, I do not understand why this
> function needs to work with such mbufs.
>
> [Yi Yang] Only DPDK driver/Tx function will use it, not OVS,
> set_multiseg_udptcp_cksum is called in GSO part, it is last step before Tx
> function, it is a big external mbuf before rte_gso_segment, that isn't a
> multi-segmented mbuf.
>
Hi Ilya,
Now I understand Yi Yang's point better and I agree with him.
Looks like the patch does the GSO at the DPDK TX function.
It creates multi-seg mbuf after rte_gso_segment(), but will immediately send
out the multi-seg mbuf to DPDK port, without traversing inside other part of
OVS code. I guess this case it should work OK?
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
