muvarov replied on github web page:
platform/linux-generic/pktio/dpdk.c
line 44
@@ -337,39 +337,62 @@ static struct rte_mempool_ops ops_stack = {
MEMPOOL_REGISTER_OPS(ops_stack);
-#define HAS_IP4_CSUM_FLAG(m, f) ((m->ol_flags & PKT_RX_IP_CKSUM_MASK) == f)
+#define IP4_CSUM_RESULT(m) (m->ol_flags & PKT_RX_IP_CKSUM_MASK)
+#define L4_CSUM_RESULT(m) (m->ol_flags & PKT_RX_L4_CKSUM_MASK)
#define HAS_L4_PROTO(m, proto) ((m->packet_type & RTE_PTYPE_L4_MASK) == proto)
-#define HAS_L4_CSUM_FLAG(m, f) ((m->ol_flags & PKT_RX_L4_CKSUM_MASK) == f)
#define PKTIN_CSUM_BITS 0x1C
static inline int pkt_set_ol_rx(odp_pktin_config_opt_t *pktin_cfg,
odp_packet_hdr_t *pkt_hdr,
struct rte_mbuf *mbuf)
{
+ uint32_t packet_csum_result;
+
if (pktin_cfg->bit.ipv4_chksum &&
- RTE_ETH_IS_IPV4_HDR(mbuf->packet_type) &&
- HAS_IP4_CSUM_FLAG(mbuf, PKT_RX_IP_CKSUM_BAD)) {
- if (pktin_cfg->bit.drop_ipv4_err)
- return -1;
+ RTE_ETH_IS_IPV4_HDR(mbuf->packet_type)) {
+ packet_csum_result = IP4_CSUM_RESULT(mbuf);
+
+ if (packet_csum_result == PKT_RX_IP_CKSUM_GOOD) {
+ pkt_hdr->p.input_flags.l3_chksum_done = 1;
+ } else if (packet_csum_result != PKT_RX_IP_CKSUM_UNKNOWN) {
+ if (pktin_cfg->bit.drop_ipv4_err)
+ return -1;
- pkt_hdr->p.error_flags.ip_err = 1;
+ pkt_hdr->p.input_flags.l3_chksum_done = 1;
+ pkt_hdr->p.error_flags.ip_err = 1;
+ pkt_hdr->p.error_flags.l3_chksum = 1;
+ }
}
if (pktin_cfg->bit.udp_chksum &&
- HAS_L4_PROTO(mbuf, RTE_PTYPE_L4_UDP) &&
- HAS_L4_CSUM_FLAG(mbuf, PKT_RX_L4_CKSUM_BAD)) {
- if (pktin_cfg->bit.drop_udp_err)
- return -1;
+ HAS_L4_PROTO(mbuf, RTE_PTYPE_L4_UDP)) {
Comment:
@bogdanPricope yes, 64 bit. On than only change for 64 is needed.
> bogdanPricope wrote
> This function should set csum related input/error flags for L3 (IPv4 packets)
> and for L4 UDP/TCP (IPv4/IPv6 packets); all flags should be set if requested
> at configuration time (pktin_cfg->bit.*). I donât see how your function is
> doing this.
> And yes, flags are uint64_t in dpdk 17.08/17.02. I will generate a new
> version for this.
>
> struct rte_mbuf {
> â¦â¦â¦â¦
> uint64_t ol_flags; /**< Offload features. */
> â¦â¦â¦â¦
> }
>> muvarov wrote
>> @bogdanPricope will that be the same and more clear
>> https://github.com/muvarov/odp/commit/c56c3663725c73e7cf1a39fb6e37341dd2f3649f
>> ? Also flags are uint16_t in my version of dpdk, not uint32_t.
>>> muvarov wrote
>>> @bogdanPricope hm, please skip that comment, logic looks like correct.
>>>> bogdanPricope wrote
>>>> error_flags.tcp_err and error_flags.udp_err are part of existing API.
>>>>> bogdanPricope wrote
>>>>> Cannot avoid the 'ifs'. Final code (not diffs) looks better and easier to
>>>>> understand.
>>>>>> bogdanPricope wrote
>>>>>> Prints were already there. We only changed the conditions for printing
>>>>>> to highlight the new API.
>>>>>>> muvarov wrote
>>>>>>> common errors set for tcp and udp here.
>>>>>>>> muvarov wrote
>>>>>>>> too many ifs overcompicate function and makes it hard to read. Tcp
>>>>>>>> packet and pktin_cfg->bit.udp_chksum=0 in settings will not to tcp
>>>>>>>> 'if' bellow.
>>>>>>>>> muvarov wrote
>>>>>>>>> prints for fast path functions is bad thing. Bellow is print_pkts()
>>>>>>>>> which does all printing. Or we need to move it here at least. Or
>>>>>>>>> better to move out all printing from fast path.
https://github.com/Linaro/odp/pull/269#discussion_r149892702
updated_at 2017-11-09 08:51:47
