* [PATCH 0/2] Two offloading issues of tep_term
@ 2016-08-04 7:58 Jianfeng Tan
2016-08-04 7:58 ` [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure Jianfeng Tan
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jianfeng Tan @ 2016-08-04 7:58 UTC (permalink / raw)
To: dev; +Cc: konstantin.ananyev, jingjing.wu, mark.b.kavanagh, Jianfeng Tan
This patch set depends on:
- http://dpdk.org/ml/archives/dev/2016-August/044924.html
Patch 1: fill tunneling type.
Patch 2: inner L4 checksum error.
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Jianfeng Tan (2):
examples/tep_term: fix offload on VXLAN failure
examples/tep_term: fix inner L4 checksum failure
examples/tep_termination/vxlan.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure 2016-08-04 7:58 [PATCH 0/2] Two offloading issues of tep_term Jianfeng Tan @ 2016-08-04 7:58 ` Jianfeng Tan 2016-09-11 12:09 ` Yuanhan Liu 2016-08-04 7:58 ` [PATCH 2/2] examples/tep_term: fix inner L4 checksum failure Jianfeng Tan 2016-10-13 9:39 ` [PATCH 0/2] Two offloading issues of tep_term Thomas Monjalon 2 siblings, 1 reply; 9+ messages in thread From: Jianfeng Tan @ 2016-08-04 7:58 UTC (permalink / raw) To: dev; +Cc: konstantin.ananyev, jingjing.wu, mark.b.kavanagh, Jianfeng Tan Based on previous fix of offload on VXLAN using i40e, applications need to set proper tunneling type on ol_flags so that i40e driver can pass it to NIC. Fixes: a50245ede72a ("examples/tep_term: initialize VXLAN sample") Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> --- examples/tep_termination/vxlan.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/tep_termination/vxlan.c b/examples/tep_termination/vxlan.c index 5ee1f95..4bad33d 100644 --- a/examples/tep_termination/vxlan.c +++ b/examples/tep_termination/vxlan.c @@ -237,6 +237,8 @@ encapsulation(struct rte_mbuf *m, uint8_t queue_id) m->outer_l2_len = sizeof(struct ether_hdr); m->outer_l3_len = sizeof(struct ipv4_hdr); + ol_flags |= PKT_TX_TUNNEL_VXLAN; + m->ol_flags |= ol_flags; m->tso_segsz = tx_offload.tso_segsz; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure 2016-08-04 7:58 ` [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure Jianfeng Tan @ 2016-09-11 12:09 ` Yuanhan Liu 2016-09-12 8:42 ` Tan, Jianfeng 0 siblings, 1 reply; 9+ messages in thread From: Yuanhan Liu @ 2016-09-11 12:09 UTC (permalink / raw) To: Jianfeng Tan Cc: dev, konstantin.ananyev, jingjing.wu, mark.b.kavanagh, Thomas Monjalon On Thu, Aug 04, 2016 at 07:58:48AM +0000, Jianfeng Tan wrote: > Based on previous fix of offload on VXLAN using i40e, applications > need to set proper tunneling type on ol_flags so that i40e driver > can pass it to NIC. > > Fixes: a50245ede72a ("examples/tep_term: initialize VXLAN sample") > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> > --- > examples/tep_termination/vxlan.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/examples/tep_termination/vxlan.c b/examples/tep_termination/vxlan.c > index 5ee1f95..4bad33d 100644 > --- a/examples/tep_termination/vxlan.c > +++ b/examples/tep_termination/vxlan.c > @@ -237,6 +237,8 @@ encapsulation(struct rte_mbuf *m, uint8_t queue_id) > m->outer_l2_len = sizeof(struct ether_hdr); > m->outer_l3_len = sizeof(struct ipv4_hdr); > > + ol_flags |= PKT_TX_TUNNEL_VXLAN; > + Hi, FYI, my testrobot caught some errors when this patch is applied. --yliu --- x86_64-native-linuxapp-clang: config-all-yes ============================================ /root/dpdk/examples/tep_termination/vxlan.c:240:14: error: use of undeclared identifier 'PKT_TX_TUNNEL_VXLAN' ol_flags |= PKT_TX_TUNNEL_VXLAN; ^ 1 error generated. make[1]: *** [vxlan.o] Error 1 make: *** [all] Error 2 error: build examples/tep_termination failed error: build failed ... ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure 2016-09-11 12:09 ` Yuanhan Liu @ 2016-09-12 8:42 ` Tan, Jianfeng 2016-09-12 9:36 ` Thomas Monjalon 0 siblings, 1 reply; 9+ messages in thread From: Tan, Jianfeng @ 2016-09-12 8:42 UTC (permalink / raw) To: Yuanhan Liu Cc: dev, Ananyev, Konstantin, Wu, Jingjing, Kavanagh, Mark B, Thomas Monjalon Hi Yuanhan, > -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > Sent: Sunday, September 11, 2016 8:09 PM > To: Tan, Jianfeng > Cc: dev@dpdk.org; Ananyev, Konstantin; Wu, Jingjing; Kavanagh, Mark B; > Thomas Monjalon > Subject: Re: [dpdk-dev] [PATCH 1/2] examples/tep_term: fix offload on > VXLAN failure > > On Thu, Aug 04, 2016 at 07:58:48AM +0000, Jianfeng Tan wrote: > > Based on previous fix of offload on VXLAN using i40e, applications > > need to set proper tunneling type on ol_flags so that i40e driver > > can pass it to NIC. > > > > Fixes: a50245ede72a ("examples/tep_term: initialize VXLAN sample") > > > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> > > --- > > examples/tep_termination/vxlan.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/examples/tep_termination/vxlan.c > b/examples/tep_termination/vxlan.c > > index 5ee1f95..4bad33d 100644 > > --- a/examples/tep_termination/vxlan.c > > +++ b/examples/tep_termination/vxlan.c > > @@ -237,6 +237,8 @@ encapsulation(struct rte_mbuf *m, uint8_t > queue_id) > > m->outer_l2_len = sizeof(struct ether_hdr); > > m->outer_l3_len = sizeof(struct ipv4_hdr); > > > > + ol_flags |= PKT_TX_TUNNEL_VXLAN; > > + > > Hi, > > FYI, my testrobot caught some errors when this patch is applied. It's because this patch set has dependency on a previous patch set, which seems a difficult scenario to handle. There's no standard to state the dependency, right? Thanks, Jianfeng > > --yliu > > --- > x86_64-native-linuxapp-clang: config-all-yes > ============================================ > /root/dpdk/examples/tep_termination/vxlan.c:240:14: error: use of > undeclared identifier 'PKT_TX_TUNNEL_VXLAN' > ol_flags |= PKT_TX_TUNNEL_VXLAN; > ^ > 1 error generated. > make[1]: *** [vxlan.o] Error 1 > make: *** [all] Error 2 > error: build examples/tep_termination failed > error: build failed > > ... ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure 2016-09-12 8:42 ` Tan, Jianfeng @ 2016-09-12 9:36 ` Thomas Monjalon 2016-09-13 0:50 ` Tan, Jianfeng 2016-09-13 2:40 ` Yuanhan Liu 0 siblings, 2 replies; 9+ messages in thread From: Thomas Monjalon @ 2016-09-12 9:36 UTC (permalink / raw) To: Tan, Jianfeng, Yuanhan Liu; +Cc: dev 2016-09-12 08:42, Tan, Jianfeng: > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > > FYI, my testrobot caught some errors when this patch is applied. > > It's because this patch set has dependency on a previous patch set, which seems a difficult scenario to handle. There's no standard to state the dependency, right? No there is no standard to state the dependency. We need one. Actually, there are 3 kinds of dependencies: - a well know dependency when sending a patch - implicit dependency on the HEAD (can fail if a conflicting patch is pushed) - dependency on a specific tree (next-*) I suggest using: Depends-on: pw <patchwork-id-of-the-patch>|<tree> <hash> Examples: Depends-on: pw 33000 Depends-on: master 3643b0f Depends-on: next-net f33e00 It won't work well when a patch depends on a pending patch series because the cover letter has no patchwork identifier. It will be solved with the next version of patchwork (in few months). In the meantime, we can point to the first patch of the series. Comments/ideas? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure 2016-09-12 9:36 ` Thomas Monjalon @ 2016-09-13 0:50 ` Tan, Jianfeng 2016-09-13 2:40 ` Yuanhan Liu 1 sibling, 0 replies; 9+ messages in thread From: Tan, Jianfeng @ 2016-09-13 0:50 UTC (permalink / raw) To: Thomas Monjalon, Yuanhan Liu; +Cc: dev Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Monday, September 12, 2016 5:36 PM > To: Tan, Jianfeng; Yuanhan Liu > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/2] examples/tep_term: fix offload on > VXLAN failure > > 2016-09-12 08:42, Tan, Jianfeng: > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > > > FYI, my testrobot caught some errors when this patch is applied. > > > > It's because this patch set has dependency on a previous patch set, which > seems a difficult scenario to handle. There's no standard to state the > dependency, right? > > No there is no standard to state the dependency. > We need one. Actually, there are 3 kinds of dependencies: > - a well know dependency when sending a patch > - implicit dependency on the HEAD > (can fail if a conflicting patch is pushed) > - dependency on a specific tree (next-*) > > I suggest using: > Depends-on: pw <patchwork-id-of-the-patch>|<tree> <hash> > Examples: > Depends-on: pw 33000 > Depends-on: master 3643b0f > Depends-on: next-net f33e00 > > It won't work well when a patch depends on a pending patch series > because the cover letter has no patchwork identifier. > It will be solved with the next version of patchwork (in few months). > In the meantime, we can point to the first patch of the series. > > Comments/ideas? I think it's a great idea which can make the work of auto tools much more easier and less false positive errors. Besides, it will improve the experience of code review. Thanks, Jianfeng ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure 2016-09-12 9:36 ` Thomas Monjalon 2016-09-13 0:50 ` Tan, Jianfeng @ 2016-09-13 2:40 ` Yuanhan Liu 1 sibling, 0 replies; 9+ messages in thread From: Yuanhan Liu @ 2016-09-13 2:40 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Tan, Jianfeng, dev On Mon, Sep 12, 2016 at 11:36:01AM +0200, Thomas Monjalon wrote: > 2016-09-12 08:42, Tan, Jianfeng: > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > > > FYI, my testrobot caught some errors when this patch is applied. > > > > It's because this patch set has dependency on a previous patch set, which seems a difficult scenario to handle. There's no standard to state the dependency, right? > > No there is no standard to state the dependency. Yes. > We need one. We could. > Actually, there are 3 kinds of dependencies: > - a well know dependency when sending a patch Not quite sure what it is. > - implicit dependency on the HEAD If the HEAD is in one of the DPDK public three (no matter which it is), no issue. My robot will try to apply a patchset to all available trees, one by one, until it succeeds. If the HEAD is a local commit (say, like this case, you made a patch based on some patches from community that haven't been applied yet), it's hard. The only way we can do is that we can do some brute force to get the right combination if no hint given. But I think of no reason to do that: it just brings complexity. So, we could either ask a hint from the author (the tag you mentioned below, or provide a git tree (the alternative I'd suggest, details go below). > (can fail if a conflicting patch is pushed) It also can be fixed quite easily: when failed, it could go few commits backwards, until a right base is found. > - dependency on a specific tree (next-*) Not an issue, as stated above. > > I suggest using: > Depends-on: pw <patchwork-id-of-the-patch>|<tree> <hash> > Examples: > Depends-on: pw 33000 This one is necessary, both for an auto tools and for some people to have a try. > Depends-on: master 3643b0f > Depends-on: next-net f33e00 I see no good reason to do that, for that's typically where patches apply. And since we have introduced sub-trees, it then should be obvious (for frequent contributors) that if you are making a patch to a PMD driver, you should grab the next-net tree and make patch there. Vice verse, if you got a PMD patch from mailing list and want to have a try, you should try to apply it to the next-net tree. Well, it may fail because the patch author doesn't know this generic rule, that he made patches based on master, you then could have a try with master. Besides, it just adds extra burden to developers: think that we have to add such tag to every patch. > > It won't work well when a patch depends on a pending patch series > because the cover letter has no patchwork identifier. > It will be solved with the next version of patchwork (in few months). > In the meantime, we can point to the first patch of the series. > > Comments/ideas? The alternative I'd suggest is to use git trees. I'm going to add the support of git-tree-based test (hopefully, I could do that this weekend). I'd suggest all frequent contributors to have its own DPDK tree (publicly, say at github, or internally, only works for intel). Every one has a local git, what you need to do is to push your local git to a remote tree. You then tell me the tree URL, my robot will fetch your tree timely. Once you have pushed something, it will start the build test. You could even get a report when it finishes, if you wish. In such case, there is no dependency at all: because the developer already fixed that if any. As stated, it's an alternative. People could choose the one they prefer. --yliu ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] examples/tep_term: fix inner L4 checksum failure 2016-08-04 7:58 [PATCH 0/2] Two offloading issues of tep_term Jianfeng Tan 2016-08-04 7:58 ` [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure Jianfeng Tan @ 2016-08-04 7:58 ` Jianfeng Tan 2016-10-13 9:39 ` [PATCH 0/2] Two offloading issues of tep_term Thomas Monjalon 2 siblings, 0 replies; 9+ messages in thread From: Jianfeng Tan @ 2016-08-04 7:58 UTC (permalink / raw) To: dev; +Cc: konstantin.ananyev, jingjing.wu, mark.b.kavanagh, Jianfeng Tan When sending packets from virtual machine which in need of TSO by hardware NIC, the inner L4 checksum is not correct on the other side of the cable. It's because get_psd_sum() depends on PKT_TX_TCP_SEG to calculate pseudo-header checksum, but currently this bit is set after the function get_psd_sum() is called. The fix is straightforward. Move the bit setting before get_psd_sum() is called. Fixes: a50245ede72a ("examples/tep_term: initialize VXLAN sample") Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> --- examples/tep_termination/vxlan.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/examples/tep_termination/vxlan.c b/examples/tep_termination/vxlan.c index 4bad33d..155415c 100644 --- a/examples/tep_termination/vxlan.c +++ b/examples/tep_termination/vxlan.c @@ -141,14 +141,17 @@ process_inner_cksums(struct ether_hdr *eth_hdr, union tunnel_offload_info *info) ethertype, ol_flags); } else if (l4_proto == IPPROTO_TCP) { tcp_hdr = (struct tcp_hdr *)((char *)l3_hdr + info->l3_len); - ol_flags |= PKT_TX_TCP_CKSUM; - tcp_hdr->cksum = get_psd_sum(l3_hdr, ethertype, - ol_flags); + /* Put PKT_TX_TCP_SEG bit setting before get_psd_sum(), because + * it depends on PKT_TX_TCP_SEG to calculate pseudo-header + * checksum. + */ if (tso_segsz != 0) { ol_flags |= PKT_TX_TCP_SEG; info->tso_segsz = tso_segsz; info->l4_len = sizeof(struct tcp_hdr); } + ol_flags |= PKT_TX_TCP_CKSUM; + tcp_hdr->cksum = get_psd_sum(l3_hdr, ethertype, ol_flags); } else if (l4_proto == IPPROTO_SCTP) { sctp_hdr = (struct sctp_hdr *)((char *)l3_hdr + info->l3_len); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Two offloading issues of tep_term 2016-08-04 7:58 [PATCH 0/2] Two offloading issues of tep_term Jianfeng Tan 2016-08-04 7:58 ` [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure Jianfeng Tan 2016-08-04 7:58 ` [PATCH 2/2] examples/tep_term: fix inner L4 checksum failure Jianfeng Tan @ 2016-10-13 9:39 ` Thomas Monjalon 2 siblings, 0 replies; 9+ messages in thread From: Thomas Monjalon @ 2016-10-13 9:39 UTC (permalink / raw) To: Jianfeng Tan; +Cc: dev, konstantin.ananyev, jingjing.wu, mark.b.kavanagh 2016-08-04 07:58, Jianfeng Tan: > This patch set depends on: > - http://dpdk.org/ml/archives/dev/2016-August/044924.html > > Patch 1: fill tunneling type. > Patch 2: inner L4 checksum error. > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> Applied, thanks ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-10-13 9:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-04 7:58 [PATCH 0/2] Two offloading issues of tep_term Jianfeng Tan 2016-08-04 7:58 ` [PATCH 1/2] examples/tep_term: fix offload on VXLAN failure Jianfeng Tan 2016-09-11 12:09 ` Yuanhan Liu 2016-09-12 8:42 ` Tan, Jianfeng 2016-09-12 9:36 ` Thomas Monjalon 2016-09-13 0:50 ` Tan, Jianfeng 2016-09-13 2:40 ` Yuanhan Liu 2016-08-04 7:58 ` [PATCH 2/2] examples/tep_term: fix inner L4 checksum failure Jianfeng Tan 2016-10-13 9:39 ` [PATCH 0/2] Two offloading issues of tep_term Thomas Monjalon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.