Hi Amir regardless of the user input, when a packet is received ah hash is computed calling parse_pkt -> parse_raw_pkt -> hash_pkt_header -> hash_pkt if the packet is not an IP packet (ARP or whatever), ip_version is 0 and your message is printed, for every packet.
Alfredo > On 1 Jun 2017, at 11:00, Amir Kaduri <[email protected]> wrote: > > Hi Alfredo, > > I'm aware that this function is called per packet, but if you think of > it deeper, then if everything is configured correctly by the user, > ip_version should give 4 or 6 only, thus it will never rich the "else" > I suggest to add and it will never actually add any redundant check > nor print per packet. The only time the warning will be printed is > only if ip_version will not be 4 or 6 and its great. It clearly warns > about a problem (whether its not set or set to anything else than 4 or > 6). > Just to make sure you got me right, the code with the fix should look like > that: > if (ip_version == 4) > hash += host_peer_a.v4 + host_peer_b.v4; > else if (ip_version == 6) > hash += (<... Omitted long condition >); > else > printk("Warning: ip_version (currently equals %d) should be 4 or 6 > only\n", ip_version); > > So again, if you take a moment to think of it, this warning should > never cost you anything if everything is configured well, but it warns > if anything went wrong. > > Thanks, > Amir > > On Thu, Jun 1, 2017 at 11:39 AM, Alfredo Cardigliano > <[email protected]> wrote: >> Hi Amir >> hash_pkt is called per-packet, we want to a void a printk per packet to >> avoid flooding dmesg and slowing doen packet processing, >> if ip_version comes from user input, sanity check should happen in userspace >> in my opinion. >> >> Regards >> Alfredo >> >>> On 1 Jun 2017, at 07:38, Amir Kaduri <[email protected]> wrote: >>> >>> Hi Alfredo, >>> >>> This is the exact location of the function: >>> https://github.com/ntop/PF_RING/blob/dev/kernel/pf_ring.c#L1794 >>> >>> Thanks, >>> Amir >>> >>> On Mon, May 29, 2017 at 7:28 PM, Alfredo Cardigliano >>> <[email protected]> wrote: >>>> Hi Amir >>>> what is the file location you are talking about? >>>> >>>> Alfredo >>>> >>>>> On 29 May 2017, at 18:23, Amir Kaduri <[email protected]> wrote: >>>>> >>>>> In function hash_pkt(), there is a if-else-if statement based on >>>>> ip_version. If ip_version is 0, the hash won't include the ipaddress. >>>>> Since the ip_version might come from the user input, I suggest adding >>>>> an "else" and issue a warning in case ip_version wasn't set. >>>>> _______________________________________________ >>>>> Ntop-misc mailing list >>>>> [email protected] >>>>> http://listgateway.unipi.it/mailman/listinfo/ntop-misc >>>> >>>> >>>> _______________________________________________ >>>> Ntop-misc mailing list >>>> [email protected] >>>> http://listgateway.unipi.it/mailman/listinfo/ntop-misc >>> _______________________________________________ >>> Ntop-misc mailing list >>> [email protected] >>> http://listgateway.unipi.it/mailman/listinfo/ntop-misc >> >> >> _______________________________________________ >> Ntop-misc mailing list >> [email protected] >> http://listgateway.unipi.it/mailman/listinfo/ntop-misc > _______________________________________________ > Ntop-misc mailing list > [email protected] > http://listgateway.unipi.it/mailman/listinfo/ntop-misc
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ Ntop-misc mailing list [email protected] http://listgateway.unipi.it/mailman/listinfo/ntop-misc
