cnet-graph: do not forward TCP control packets to chnl_callback

[jalalmostafa]

CNET-Graph will send TCP control packets sometimes e.g. SYN, FIN, ACK, etc. We do not need to notify these packets. Instead, we only notify packets that have user data. For UDP, we notify all.

Let me know your comments how the decision to forward the packet to chnl_callback can be improved. Right now I check data length. How about we check PSH or URG flags? If they are set in the tcb, then the packet has user data and we forward to chnl_callback. I am rethinking it because I am thinking of traffic generators that generate empty segments (where data len is zero), we may still need to get notifications in chnl_callback for these.

[KeithWiles]

I am concerned if we restrict some TCP packets from going to the callback the TCP protocol will not be handled correctly by the Kernel stack. It is valid to have zero data TCP packets with flags like when closing or other reasons.

The PR is also addressing a traffic generator or DOS attack use case.

[jalalmostafa]

Hi @KeithWiles Thanks for the feedback. That's true but closing is handled elsewhere in the chnl_callback (CHNL_UDP_CLOSE_TYPE and CHNL_TCP_CLOSE_TYPE). I think all the other conditions should either be handled using dedicated callback types if they are needed or just be handled inside the CNET stack without user intervention just like in the Linux kernel. This was also the usual case before adding the CHNL_*_SENT_TYPE callback types.

We need to cover the traffic generator/DOS use cases as you said as they usually have user data of size zero.

[KeithWiles]

Could you please run the 'graph dot` command and paste one of the text files here. I want to see the graph layout again.

Still thinking about this change, but so far I think it maybe OK.

[jalalmostafa]

digraph cndp_tcpecho_graph_1 {
    rankdir=LD; bgcolor=mistyrose
    label=<<font color='black'>Graph Name: </font><font color='blue' point-size='20'> <b>cndp_tcpecho_graph_1</b></font>      <font color='black'><b>(pkt_drop removed)</b></font>>
    edge [color=blue, arrowsize=0.6]
    node [margin=0.1 fontcolor=black fontsize=16 width=0.8 shape=box color=black style="filled,rounded"]
    {"eth_rx-0" [fillcolor=lavender]}->"ptype"
    {"eth_tx-0" [fillcolor=cyan]}->"chnl_callback"
    {"eth_rx-1" [fillcolor=lavender]}->"ptype"
    {"eth_tx-1" [fillcolor=cyan]}->"chnl_callback"
    {"ip4_input" [fillcolor=mediumspringgreen]}->{"ip4_forward" "ip4_proto"}
    {"ip4_output" [fillcolor=mediumspringgreen]}->{"arp_request" "eth_tx-0" "eth_tx-1"}
    {"ip4_forward" [fillcolor=mediumspringgreen]}->{"arp_request" "eth_tx-0" "eth_tx-1"}
    {"ip4_proto" [fillcolor=mediumspringgreen]}->{"udp_input" "tcp_input"}
    {"udp_input" [fillcolor=cornsilk]}->{"chnl_recv" "punt_kernel"}
    {"udp_output" [fillcolor=cornsilk]}->"ip4_output"
    {"tcp_input" [fillcolor=lightpink]}->{"chnl_recv" "punt_kernel"}
    {"tcp_output" [fillcolor=lightpink]}->"ip4_output"
    {"chnl_callback" [fillcolor=lightgrey]}
    {"chnl_recv" [fillcolor=yellowgreen]}->"chnl_callback"
    {"ptype" [fillcolor=goldenrod]}->{"punt_kernel" "punt_l2_kernel" "ip4_input" "gtpu_input"}
    {"arp_request" [fillcolor=mediumspringgreen]}
    {"punt_kernel" [fillcolor=coral]}
    {"punt_l2_kernel" [fillcolor=coral]}
    {"gtpu_input" [fillcolor=lightskyblue]}->"ip4_input"
}

[KeithWiles]

Thanks for the picture.

I was thinking about the PSH and URG flag test, you can have PSH (I believe) without user data and seems like checking only the size is reasonable.

[jalalmostafa]

Thanks @KeithWiles !