<div style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div style="margin: 0;">Hi <span style="white-space: pre-wrap; font-family: arial; word-break: break-word !important;">Konstantin,</span></div><div style="margin: 0;"><span style="white-space: pre-wrap; font-family: arial; word-break: break-word !important;"><pre class="content" style="box-sizing: border-box; overflow: auto; font-family: Menlo, Monaco, Consolas, "Courier New", monospace; font-size: 13px; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; color: rgb(51, 51, 51); word-break: break-all; border: 0px; border-radius: 0px;">> These macros are dups for what we have in rte_ipv4_fragmentation.c
> Would probably make sense to name them RTE_IPV4_IPOPT_... and put them
> in some public .h to avoid duplication.</pre></span></div><div style="margin: 0;"><span style="white-space: pre-wrap; font-family: arial; word-break: break-word !important;" class="">I named them RTE_IPV4_IPOPT_xxx and put them in "rte_ip_frag.h".</span></div><div style="margin: 0;"><span style="font-family: arial; white-space: pre-wrap;"><br></span></div><div style="margin: 0;"><pre class="content" style="box-sizing: border-box; overflow: auto; font-family: Menlo, Monaco, Consolas, "Courier New", monospace; font-size: 13px; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; color: rgb(51, 51, 51); word-break: break-all; border: 0px; border-radius: 0px;">> Could you clarify what this macro does?
> BTW, I assume it is a local one?
> If so, no need for RTE_ prefix.</pre><pre class="content" style="box-sizing: border-box; overflow: auto; font-family: Menlo, Monaco, Consolas, "Courier New", monospace; font-size: 13px; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; color: rgb(51, 51, 51); word-break: break-all; border: 0px; border-radius: 0px;"><span style="white-space: pre-wrap; font-family: arial; word-break: break-word !important;" class="">Yes,it is a local macro.I will cancel the RTE_ prefix.It is a toggle switch used as a different way to assemble frag test data.It is convenient for users to use different ways to assemble test data.</span></pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><pre class="content" style="color: rgb(51, 51, 51); font-family: Menlo, Monaco, Consolas, "Courier New", monospace; font-size: 13px; box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;">> Instead of returning void and having out parameter (ipopt_len),
> why just not make it a return value?
> static inline uint16_t
> __create_ipopt_frag_hdr(const uint8_t *iph, uint8_t * ipopt_frag_hdr, uint16_t len)
> {
> ....
> return ipopt_len;
> }</pre><pre class="content" style="color: rgb(51, 51, 51); font-family: Menlo, Monaco, Consolas, "Courier New", monospace; font-size: 13px; box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;">> We probably can update ihl once at the very end of this function.</pre><pre class="content" style="color: rgb(51, 51, 51); font-family: Menlo, Monaco, Consolas, "Courier New", monospace; font-size: 13px; box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;">Ok,I will modify it this way,Thank you for your advice.</pre><pre class="content" style="color: rgb(51, 51, 51); font-family: Menlo, Monaco, Consolas, "Courier New", monospace; font-size: 13px; box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;">> Can we probably do that before the loop (as we have to do it only once anyway?
> Something like:
> ....
> ipopt_len = header_len - sizeof(struct rte_ipv4_hdr);
> if (ipopt_len > RTE_IPOPT_MAX_LEN)
> return -EINVAL;
> if (ipopt_len != 0)
> ipopt_len = __create_ipopt_frag_hdr((in_hdr, ipopt_frag_hdr, ipopt_len);
> ....
> And then:
> while (likely(more_in_segs)) {
> ...
> if (ipopt_len ! = 0)
> in_hdr = (struct rte_ipv4_hdr *)ipopt_frag_hdr;
> }</pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><font color="#333333" face="Menlo, Monaco, Consolas, Courier New, monospace"><span style="font-size: 13px;">The modified code is as follows£º</span></font></pre><font color="#333333" face="Menlo, Monaco, Consolas, Courier New, monospace"><span style="font-size: 13px;"> ipopt_len = header_len - sizeof(struct rte_ipv4_hdr);
if (unlikely(ipopt_len > RTE_IPV4_IPOPT_MAX_LEN))
return -EINVAL;
else if (ipopt_len == 0)
/* Used to mark without processing frag. */
ipopt_len = RTE_IPV4_IPOPT_MAX_LEN + 1;
/* The first frag. */
else if ((flag_offset & RTE_IPV4_HDR_OFFSET_MASK) == 0)
/* Create a separate IP header to handle frag options. */
ipopt_len = __create_ipopt_frag_hdr((uint8_t *)in_hdr,
ipopt_len, ipopt_frag_hdr);
</span></font><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><font color="#333333" face="Menlo, Monaco, Consolas, Courier New, monospace"><span style="font-size: 13px;"><br>while (likely(more_in_segs)) {</span></font></pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><font color="#333333" face="Menlo, Monaco, Consolas, Courier New, monospace"><span style="font-size: 13px;"> ...</span></font></pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><font color="#333333" face="Menlo, Monaco, Consolas, Courier New, monospace"><span style="font-size: 13px;"> if (unlikely((fragment_offset == 0) &&
(ipopt_len <= RTE_IPV4_IPOPT_MAX_LEN) &&
((flag_offset & RTE_IPV4_HDR_OFFSET_MASK) == 0))) {
fragment_offset = (uint16_t)(fragment_offset +
out_pkt->pkt_len - header_len);
out_pkt->l3_len = header_len;
header_len = sizeof(struct rte_ipv4_hdr) + ipopt_len;
in_hdr = (struct rte_ipv4_hdr *)ipopt_frag_hdr;
} else {
fragment_offset = (uint16_t)(fragment_offset +
out_pkt->pkt_len - header_len);
out_pkt->l3_len = header_len;
}
</span></font></pre><div><font color="#333333" face="Menlo, Monaco, Consolas, Courier New, monospace"><span style="font-size: 13px;"><br></span></font></div><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><font color="#333333" face="Menlo, Monaco, Consolas, Courier New, monospace"><span style="font-size: 13px;">}</span></font></pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><span style="font-size: 14px;">These two pieces of code were previously merged together.It doesn't look as brief as before.I would like to hear from you.</span></pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><span style="font-size: 14px;">Some minor issues£º</span></pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><span style="font-size: 14px;">1. There are some RTE_ prefixes in the rte_ipv4_fragmentation.c.Do I need to move to a public header file?</span></pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><span style="font-size: 14px;"> /* Fragment Offset */</span></pre>#define RTE_IPV4_HDR_DF_SHIFT 14
#define RTE_IPV4_HDR_MF_SHIFT 13
#define RTE_IPV4_HDR_FO_SHIFT 3
</pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;">2. Some comments are in the following format£º/**< xxx */,What does this symbol(**<) mean?</pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;"><br></pre><pre class="content" style="box-sizing: border-box; overflow: auto; padding: 9.5px; margin-top: 0px; margin-bottom: 10px; line-height: 14.3px; word-break: break-all; border: 0px; border-radius: 0px;">Huichao,Cai</pre></div></div><br><br><span title="neteasefooter"><p> </p></span>