Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DPI related bug-fixes #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/npf/dpi/ndpi.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ dpi_ndpi_session_flow_destroy(struct dpi_engine_flow *dpi_flow)
static int
dpi_ndpi_session_first_packet(struct npf_session *se __unused,
struct npf_cache *npc __unused, struct rte_mbuf *mbuf,
int dir, uint32_t data_len, struct dpi_engine_flow **dpi_flow)
int dir, uint32_t data_len __unused,
struct dpi_engine_flow **dpi_flow)
{
struct ndpi_flow *flow = zmalloc_aligned(sizeof(struct ndpi_flow));
if (!flow)
Expand All @@ -315,8 +316,7 @@ dpi_ndpi_session_first_packet(struct npf_session *se __unused,
if (!flow->dest_id)
goto dest_id_error;

if (data_len != 0 && !dpi_ndpi_process_pkt(
(struct dpi_engine_flow *)flow, mbuf, dir))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dfawcus

In dpi.c:dpi_get_data_len() there is a comment around where the UDP length is validated, intended to simply cause packets to be dropped if the length is invalid.

The above change would seem to make the call graph to here broken, in that such packets now will not be dropped, so there would need to be another way of dealing with that case.

It is also worth keeping in mind that (eventually) UDP packets will have the possibility of having 0 traditional data, while actually carrying elsewhere because of the UDP-Options draft which is in progress.

It strikes me that the data_length field should maybe be removed from the prototype for this function, as it arose when the mbuf was not passed in, and we passed instead a data+length tuple. It is rather superfluous at the moment, as it can be extracted from the mbuf - see dpi_get_data_length().

if (!dpi_ndpi_process_pkt((struct dpi_engine_flow *)flow, mbuf, dir))
return -EINVAL;

*dpi_flow = (struct dpi_engine_flow *)flow;
Expand Down