Ok, thanks.
On Thu, Jun 1, 2017 at 12:16 PM, Alfredo Cardigliano
<cardigli...@ntop.org> wrote:
> 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 <akadur...@gmail.com> 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
>> <cardigli...@ntop.org> 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 <akadur...@gmail.com> 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
>>>> <cardigli...@ntop.org> wrote:
>>>>> Hi Amir
>>>>> what is the file location you are talking about?
>>>>>
>>>>> Alfredo
>>>>>
>>>>>> On 29 May 2017, at 18:23, Amir Kaduri <akadur...@gmail.com> 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
>>>>>> Ntop-misc@listgateway.unipi.it
>>>>>> http://listgateway.unipi.it/mailman/listinfo/ntop-misc
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Ntop-misc mailing list
>>>>> Ntop-misc@listgateway.unipi.it
>>>>> http://listgateway.unipi.it/mailman/listinfo/ntop-misc
>>>> _______________________________________________
>>>> Ntop-misc mailing list
>>>> Ntop-misc@listgateway.unipi.it
>>>> http://listgateway.unipi.it/mailman/listinfo/ntop-misc
>>>
>>>
>>> _______________________________________________
>>> Ntop-misc mailing list
>>> Ntop-misc@listgateway.unipi.it
>>> http://listgateway.unipi.it/mailman/listinfo/ntop-misc
>> _______________________________________________
>> Ntop-misc mailing list
>> Ntop-misc@listgateway.unipi.it
>> http://listgateway.unipi.it/mailman/listinfo/ntop-misc
>
>
> _______________________________________________
> Ntop-misc mailing list
> Ntop-misc@listgateway.unipi.it
> http://listgateway.unipi.it/mailman/listinfo/ntop-misc
_______________________________________________
Ntop-misc mailing list
Ntop-misc@listgateway.unipi.it
http://listgateway.unipi.it/mailman/listinfo/ntop-misc
