FromDPDKDevice() doesn't set_mac_header

[kkovacs]

Hi Click team,

first of all, thanks for the great software.

While developing a custom Element, I've noticed that while most source elements set the originated packets' mac header address with something along the lines of p->set_mac_header(p->data());, FromDPDKDevice doesn't.

Is there a rationale behind this that I don't see (performance, memory, etc), or is it merely an oversight?

For example, in elements/userlevel/fromdevice.cc:

FromDevice::emit_packet(WritablePacket *p, int extra_len, const Timestamp &ts)
{
    [...]
    p->set_packet_type_anno(Packet::MULTICAST);
    [...]
    // set annotations
    p->set_timestamp_anno(ts);
    p->set_mac_header(p->data());

while in elements/userlevel/fromdpdkdevice.cc, it's only:

bool FromDPDKDevice::run_task(Task * t)
    [...]
    p->set_packet_type_anno(Packet::HOST);

It would be good to have the mac_header set, since then one could const click_ether *eth = p_in->mac_header(); independently of what kind of source the packet came through (FromDevice(), FromDPDKDevice(), PCAP...)

I'm happy to submit a patch if the community says that it would be OK to set it.

Keep up the good work!

[tbarbette]

Hi,
You're correct the MAC header should be set. Meanwhile you can use MarkMACHeader element to set it. You can do a PR, yes.

We may want to add also the network and transport headers that are directly given by the NIC when possible. I'll look at that in a second time.

Regarding the packet type anno, I remember copying what was done for the Netmap mode. I'm not sure if the behaviour of that should change. Any idea? I'm not sure where it is used and why.

[kkovacs]

Thanks for the reply Tom, I will send a PR later today or tomorrow the latest.

Network and transport headers that are directly given by the NIC would be great, thanks for looking into that later!

I'm not sure about the packet type either, I'm fairly new to the codebase :) Will try to look into that a little.

Keep up the good work!

[kkovacs]

Hi Tom,

submitted the same change in click and fastclick also (actually I'm using fastclick, I only reported in click since the issue came from the parent project), since I don't know which one is easier for you.

Feel free to reject one of the PRs, or however you like to do it.

It's probably the simplest patch ever, but as Deep Thought said in the HGTG, at least it's very well tested.

Keep up the good work, and thanks! :)

K.

[tbarbette]

Thanks !
However you did the pull request towards your own master. Close yours and re-open only one towards kohler/master. I'll take care of merging inside FastClick ;)

[kkovacs]

Damn, I'm new to this. Ok, will do in a moment :)