Message ID | 20230421010932.46847-1-cheng1.jiang@intel.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8F8D7429A2; Fri, 21 Apr 2023 03:44:29 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1BB56410DD; Fri, 21 Apr 2023 03:44:29 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mails.dpdk.org (Postfix) with ESMTP id A093F40E6E for <dev@dpdk.org>; Fri, 21 Apr 2023 03:44:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1682041467; x=1713577467; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=ww4vusGr/rFzVQcj/bz10Mi+bq85p+R2ju0eXRhFjpo=; b=GBGOFezs3pNHzDTnkL1tK6Y1HvHfzxTxHr//hglf6IXJjN0lL9rVTjq8 TfSy7QaEZWy3dXSDsuNgtPt3stX6JBBDU0OuR8AjzOFnbgRiIWxxSBrmC /xLY10MZ7G5+aVlSZn/3uzgGfiFI0Xahi3vQGFEjqCUbeb+29FTgr58lJ rI7Lhl0FkSbkA9tx0fMGVHeqb2HbLebrqLIWg8eZdypR8Zx98sRPhGv/E oqyAFib28cmaRAaCQCLUZszS9JQSxsEeM6JrXpzB2CDANsNgTQ47cvpF4 8j3lcKuVJaHY4x3eR7wBx/LjDQytvY5P5ElNMTpuFNuScLqn9JpWb9NUy A==; X-IronPort-AV: E=McAfee;i="6600,9927,10686"; a="334763675" X-IronPort-AV: E=Sophos;i="5.99,214,1677571200"; d="scan'208";a="334763675" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Apr 2023 18:44:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10686"; a="669558549" X-IronPort-AV: E=Sophos;i="5.99,214,1677571200"; d="scan'208";a="669558549" Received: from dpdk_jiangcheng.sh.intel.com ([10.67.119.139]) by orsmga006.jf.intel.com with ESMTP; 20 Apr 2023 18:44:24 -0700 From: Cheng Jiang <cheng1.jiang@intel.com> To: maxime.coquelin@redhat.com, chenbo.xia@intel.com Cc: dev@dpdk.org, jiayu.hu@intel.com, xuan.ding@intel.com, wenwux.ma@intel.com, yuanx.wang@intel.com, xingguang.he@intel.com, Cheng Jiang <cheng1.jiang@intel.com> Subject: [PATCH 0/2] vhost: add port mirroring function in the vhost lib Date: Fri, 21 Apr 2023 01:09:30 +0000 Message-Id: <20230421010932.46847-1-cheng1.jiang@intel.com> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org |
Series |
vhost: add port mirroring function in the vhost lib
|
|
Message
Jiang, Cheng1
April 21, 2023, 1:09 a.m. UTC
Similar to the port mirroring function on the switch or router, this patch set implements such function on the Vhost lib. When data is sent to a front-end, it will also send the data to its mirror front-end. When data is received from a front-end, it will also send the data to its mirror front-end. Cheng Jiang (2): vhost: add ingress API for port mirroring datapath vhost: add egress API for port mirroring datapath lib/vhost/rte_vhost_async.h | 17 + lib/vhost/version.map | 3 + lib/vhost/virtio_net.c | 1266 +++++++++++++++++++++++++++++++++++ 3 files changed, 1286 insertions(+) -- 2.35.1
Comments
Hi Cheng, On 4/21/23 03:09, Cheng Jiang wrote: > Similar to the port mirroring function on the switch or router, this > patch set implements such function on the Vhost lib. When > data is sent to a front-end, it will also send the data to its mirror > front-end. When data is received from a front-end, it will also send > the data to its mirror front-end. Why not just keeping mirroring in the switch/router? I am really not convinced this is the way to go: 1. API is too complex 2. It requires async support 3. There is too much code duplication, it increases virtio-net.c by 30%, and it is without packed ring support. 4. If mirror port is down for any reason, packets to/from the original port are dropped. 5. It seems to assume negotiated features of the two ports are identical, e.g. Virtio-net header length? If so, that's not a manageable solution. Regards, Maxime > > Cheng Jiang (2): > vhost: add ingress API for port mirroring datapath > vhost: add egress API for port mirroring datapath > > lib/vhost/rte_vhost_async.h | 17 + > lib/vhost/version.map | 3 + > lib/vhost/virtio_net.c | 1266 +++++++++++++++++++++++++++++++++++ > 3 files changed, 1286 insertions(+) > > -- > 2.35.1 >
Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Wednesday, May 3, 2023 5:37 PM > To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo > <chenbo.xia@intel.com> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan > <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang, YuanX > <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; David > Marchand <david.marchand@redhat.com> > Subject: Re: [PATCH 0/2] vhost: add port mirroring function in the vhost lib > > Hi Cheng, > > On 4/21/23 03:09, Cheng Jiang wrote: > > Similar to the port mirroring function on the switch or router, this > > patch set implements such function on the Vhost lib. When data is sent > > to a front-end, it will also send the data to its mirror front-end. > > When data is received from a front-end, it will also send the data to > > its mirror front-end. > > Why not just keeping mirroring in the switch/router? > I am really not convinced this is the way to go: > 1. API is too complex > 2. It requires async support > 3. There is too much code duplication, it increases virtio-net.c by > 30%, and it is without packed ring support. > 4. If mirror port is down for any reason, packets to/from the original > port are dropped. > 5. It seems to assume negotiated features of the two ports are > identical, e.g. Virtio-net header length? If so, that's not a > manageable solution. Thank you for your feedback. I concur that placing the mirror function in the Vhost library is not ideal. We are currently considering implementing either a mirror Vhost PMD, or adding a function to TestPMD to handle this functionality. Would you please share your thoughts on this plan and let us know which option you prefer? Thanks a lot, Cheng > > Regards, > Maxime > > > > > Cheng Jiang (2): > > vhost: add ingress API for port mirroring datapath > > vhost: add egress API for port mirroring datapath > > > > lib/vhost/rte_vhost_async.h | 17 + > > lib/vhost/version.map | 3 + > > lib/vhost/virtio_net.c | 1266 +++++++++++++++++++++++++++++++++++ > > 3 files changed, 1286 insertions(+) > > > > -- > > 2.35.1 > >
> -----Original Message----- > From: Jiang, Cheng1 <cheng1.jiang@intel.com> > Sent: Monday, May 8, 2023 8:23 PM > To: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo > <chenbo.xia@intel.com> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan > <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang, YuanX > <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; David > Marchand <david.marchand@redhat.com> > Subject: RE: [PATCH 0/2] vhost: add port mirroring function in the vhost > lib > > Hi Maxime, > > > -----Original Message----- > > From: Maxime Coquelin <maxime.coquelin@redhat.com> > > Sent: Wednesday, May 3, 2023 5:37 PM > > To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo > > <chenbo.xia@intel.com> > > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan > > <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang, YuanX > > <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; David > > Marchand <david.marchand@redhat.com> > > Subject: Re: [PATCH 0/2] vhost: add port mirroring function in the vhost > lib > > > > Hi Cheng, > > > > On 4/21/23 03:09, Cheng Jiang wrote: > > > Similar to the port mirroring function on the switch or router, this > > > patch set implements such function on the Vhost lib. When data is sent > > > to a front-end, it will also send the data to its mirror front-end. > > > When data is received from a front-end, it will also send the data to > > > its mirror front-end. > > > > Why not just keeping mirroring in the switch/router? > > I am really not convinced this is the way to go: > > 1. API is too complex > > 2. It requires async support > > 3. There is too much code duplication, it increases virtio-net.c by > > 30%, and it is without packed ring support. > > 4. If mirror port is down for any reason, packets to/from the original > > port are dropped. > > 5. It seems to assume negotiated features of the two ports are > > identical, e.g. Virtio-net header length? If so, that's not a > > manageable solution. > > Thank you for your feedback. > I concur that placing the mirror function in the Vhost library is not > ideal. We are currently considering implementing either a mirror Vhost PMD, > or adding a function to TestPMD to handle this functionality. > Would you please share your thoughts on this plan and let us know which > option you prefer? Based on current implementation, it seems that vhost lib could be ignorant of the mirroring usage. Making these logic into APP like testpmd seems to make more sense. Thanks, Chenbo > > Thanks a lot, > Cheng > > > > > > > Regards, > > Maxime > > > > > > > > Cheng Jiang (2): > > > vhost: add ingress API for port mirroring datapath > > > vhost: add egress API for port mirroring datapath > > > > > > lib/vhost/rte_vhost_async.h | 17 + > > > lib/vhost/version.map | 3 + > > > lib/vhost/virtio_net.c | 1266 > +++++++++++++++++++++++++++++++++++ > > > 3 files changed, 1286 insertions(+) > > > > > > -- > > > 2.35.1 > > >
On 5/11/23 10:59, Xia, Chenbo wrote: >> -----Original Message----- >> From: Jiang, Cheng1 <cheng1.jiang@intel.com> >> Sent: Monday, May 8, 2023 8:23 PM >> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo >> <chenbo.xia@intel.com> >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan >> <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang, YuanX >> <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; David >> Marchand <david.marchand@redhat.com> >> Subject: RE: [PATCH 0/2] vhost: add port mirroring function in the vhost >> lib >> >> Hi Maxime, >> >>> -----Original Message----- >>> From: Maxime Coquelin <maxime.coquelin@redhat.com> >>> Sent: Wednesday, May 3, 2023 5:37 PM >>> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo >>> <chenbo.xia@intel.com> >>> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan >>> <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang, YuanX >>> <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; David >>> Marchand <david.marchand@redhat.com> >>> Subject: Re: [PATCH 0/2] vhost: add port mirroring function in the vhost >> lib >>> >>> Hi Cheng, >>> >>> On 4/21/23 03:09, Cheng Jiang wrote: >>>> Similar to the port mirroring function on the switch or router, this >>>> patch set implements such function on the Vhost lib. When data is sent >>>> to a front-end, it will also send the data to its mirror front-end. >>>> When data is received from a front-end, it will also send the data to >>>> its mirror front-end. >>> >>> Why not just keeping mirroring in the switch/router? >>> I am really not convinced this is the way to go: >>> 1. API is too complex >>> 2. It requires async support >>> 3. There is too much code duplication, it increases virtio-net.c by >>> 30%, and it is without packed ring support. >>> 4. If mirror port is down for any reason, packets to/from the original >>> port are dropped. >>> 5. It seems to assume negotiated features of the two ports are >>> identical, e.g. Virtio-net header length? If so, that's not a >>> manageable solution. >> >> Thank you for your feedback. >> I concur that placing the mirror function in the Vhost library is not >> ideal. We are currently considering implementing either a mirror Vhost PMD, >> or adding a function to TestPMD to handle this functionality. >> Would you please share your thoughts on this plan and let us know which >> option you prefer? > > Based on current implementation, it seems that vhost lib could be ignorant > of the mirroring usage. Making these logic into APP like testpmd seems to make > more sense. I agree with Chenbo, it should be done at the application level, which would enable to also mirror non-Vhost ports. Thanks, Maxime > Thanks, > Chenbo > >> >> Thanks a lot, >> Cheng >> >> >> >>> >>> Regards, >>> Maxime >>> >>>> >>>> Cheng Jiang (2): >>>> vhost: add ingress API for port mirroring datapath >>>> vhost: add egress API for port mirroring datapath >>>> >>>> lib/vhost/rte_vhost_async.h | 17 + >>>> lib/vhost/version.map | 3 + >>>> lib/vhost/virtio_net.c | 1266 >> +++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 1286 insertions(+) >>>> >>>> -- >>>> 2.35.1 >>>> >
Hi Maxime/Chenbo, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Thursday, May 11, 2023 8:16 PM > To: Xia, Chenbo <chenbo.xia@intel.com>; Jiang, Cheng1 > <cheng1.jiang@intel.com> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan > <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang, YuanX > <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; David > Marchand <david.marchand@redhat.com> > Subject: Re: [PATCH 0/2] vhost: add port mirroring function in the vhost lib > > > > On 5/11/23 10:59, Xia, Chenbo wrote: > >> -----Original Message----- > >> From: Jiang, Cheng1 <cheng1.jiang@intel.com> > >> Sent: Monday, May 8, 2023 8:23 PM > >> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo > >> <chenbo.xia@intel.com> > >> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan > >> <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang, > YuanX > >> <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; David > >> Marchand <david.marchand@redhat.com> > >> Subject: RE: [PATCH 0/2] vhost: add port mirroring function in the > >> vhost lib > >> > >> Hi Maxime, > >> > >>> -----Original Message----- > >>> From: Maxime Coquelin <maxime.coquelin@redhat.com> > >>> Sent: Wednesday, May 3, 2023 5:37 PM > >>> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo > >>> <chenbo.xia@intel.com> > >>> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan > >>> <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang, > YuanX > >>> <yuanx.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; > >>> David Marchand <david.marchand@redhat.com> > >>> Subject: Re: [PATCH 0/2] vhost: add port mirroring function in the > >>> vhost > >> lib > >>> > >>> Hi Cheng, > >>> > >>> On 4/21/23 03:09, Cheng Jiang wrote: > >>>> Similar to the port mirroring function on the switch or router, > >>>> this patch set implements such function on the Vhost lib. When data > >>>> is sent to a front-end, it will also send the data to its mirror front-end. > >>>> When data is received from a front-end, it will also send the data > >>>> to its mirror front-end. > >>> > >>> Why not just keeping mirroring in the switch/router? > >>> I am really not convinced this is the way to go: > >>> 1. API is too complex > >>> 2. It requires async support > >>> 3. There is too much code duplication, it increases virtio-net.c by > >>> 30%, and it is without packed ring support. > >>> 4. If mirror port is down for any reason, packets to/from the original > >>> port are dropped. > >>> 5. It seems to assume negotiated features of the two ports are > >>> identical, e.g. Virtio-net header length? If so, that's not a > >>> manageable solution. > >> > >> Thank you for your feedback. > >> I concur that placing the mirror function in the Vhost library is not > >> ideal. We are currently considering implementing either a mirror > >> Vhost PMD, or adding a function to TestPMD to handle this functionality. > >> Would you please share your thoughts on this plan and let us know > >> which option you prefer? > > > > Based on current implementation, it seems that vhost lib could be > > ignorant of the mirroring usage. Making these logic into APP like > > testpmd seems to make more sense. > > I agree with Chenbo, it should be done at the application level, which would > enable to also mirror non-Vhost ports. I mostly agree with your opinions. However, the workload for implementing a general mirror framework to support all ports in the application level would be too large for us for now. Currently, we are more focused on demonstrating the superior performance advantages of async Vhost and DSA with this mirror work. Therefore, we may need to temporarily put this upstream process on hold. I greatly appreciate your feedback and help. Maybe we can discuss the details in the future. Thank you again! Best regards, Cheng > > Thanks, > Maxime > > > Thanks, > > Chenbo > > > >> > >> Thanks a lot, > >> Cheng > >> > >> > >> > >>> > >>> Regards, > >>> Maxime > >>> > >>>> > >>>> Cheng Jiang (2): > >>>> vhost: add ingress API for port mirroring datapath > >>>> vhost: add egress API for port mirroring datapath > >>>> > >>>> lib/vhost/rte_vhost_async.h | 17 + > >>>> lib/vhost/version.map | 3 + > >>>> lib/vhost/virtio_net.c | 1266 > >> +++++++++++++++++++++++++++++++++++ > >>>> 3 files changed, 1286 insertions(+) > >>>> > >>>> -- > >>>> 2.35.1 > >>>> > >