On 07-Jul-23 4:12 AM, Xiao Liang wrote:
|<- mb->pkt_len - sqh_len ->|
|<- sa->hdr_len ->|
|<- sa->hdr_l3_off ->| |<- udph->dgram_len ->|
+--------------------+------------+-----+-----+---------+-----+
| ETH | IP | UDP | ESP | payload | sqh |
+--------------------+------------+-----+-----+---------+-----+
|<- sa->hdr_l3_off ->|<- l3_len ->|
|<- prm->tun.hdr_len ->|
|<- sa->hdr_len ->|
sa->hdr_len and prm->tun.hdr_len don't include L2 length so both should
start in the diagram at the end of the ETH header.
So the right way to compute datagram length is
dgram_len = mb->pkt_len - sqh_len - sa->hdr_l3_off - sa->hdr_len +
sizeof(struct rte_udp_hdr)
The figure above shows how
udph->dgram_len = mb->pkt_len - sqh_len - sa->hdr_len +
sizeof(struct rte_udp_hdr);
and
l3_len = prm->tun.hdr_len - sa->hdr_l3_off;
Correct me if anything wrong.
Thanks,
Xiao Liang
On Thu, Jul 6, 2023 at 6:20 PM Radu Nicolau <radu.nico...@intel.com> wrote:
On 06-Jul-23 10:08 AM, Konstantin Ananyev wrote:
Hi Akhil,
Hi Konstantin,
Can you review this patch?
UDP header length is included in sa->hdr_len. Take care of that in
L3 header and pakcet length calculation.
Fixes: 01eef5907fc3 ("ipsec: support NAT-T")
Signed-off-by: Xiao Liang <shaw.l...@gmail.com>
---
lib/ipsec/esp_outb.c | 2 +-
lib/ipsec/sa.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/ipsec/esp_outb.c b/lib/ipsec/esp_outb.c
index 9cbd9202f6..ec87b1dce2 100644
--- a/lib/ipsec/esp_outb.c
+++ b/lib/ipsec/esp_outb.c
@@ -198,7 +198,7 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa,
rte_be64_t sqc,
struct rte_udp_hdr *udph = (struct rte_udp_hdr *)
(ph + sa->hdr_len - sizeof(struct rte_udp_hdr));
udph->dgram_len = rte_cpu_to_be_16(mb->pkt_len - sqh_len -
- sa->hdr_l3_off - sa->hdr_len);
+ sa->hdr_len + sizeof(struct rte_udp_hdr));
To be honest, it is not clear to me why we shouldn't take into account
sa->hdr_l3_off
any more.
Probably the author can explain.
Also would like author of NAT-T support to chime in.
Radu, any comments on that patch?
I agree, hdr_l3_off should not be ignored. Also sa->hdr_len already
includes the size of UDP header, see line 366 in esp_outb_tun_init in
sa.c (or the line above this change, where the udph pointer is computed
assuming this)
Thanks
Konstantin
}
/* update original and new ip header fields */
diff --git a/lib/ipsec/sa.c b/lib/ipsec/sa.c
index 59a547637d..2297bd6d72 100644
--- a/lib/ipsec/sa.c
+++ b/lib/ipsec/sa.c
@@ -371,7 +371,7 @@ esp_outb_tun_init(struct rte_ipsec_sa *sa, const struct
rte_ipsec_sa_prm *prm)
/* update l2_len and l3_len fields for outbound mbuf */
sa->tx_offload.val = rte_mbuf_tx_offload(sa->hdr_l3_off,
- sa->hdr_len - sa->hdr_l3_off, 0, 0, 0, 0, 0);
+ prm->tun.hdr_len - sa->hdr_l3_off, 0, 0, 0, 0, 0);
esp_outb_init(sa, sa->hdr_len, prm->ipsec_xform.esn.value);
}
--
2.40.0
