On Fri, Jul 13, 2012 at 04:24:31PM +0200, Nikola Pajkovsky wrote:
Vitezslav Samel <vitezslav(a)samel.cz> writes:
> On Fri, Jul 13, 2012 at 03:19:18PM +0200, Nikola Pajkovsky wrote:
>> not everything is possible converth right now
>>
>> Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com>
>> ---
>> src/detstats.c | 6 ++---
>> src/itrafmon.c | 70 +++++++++++++++++++++++++-------------------------------
>> src/packet.c | 6 +++++
>> src/packet.h | 4 ++++
>> src/serv.c | 13 +++++------
>> 5 files changed, 49 insertions(+), 50 deletions(-)
>>
>> diff --git a/src/packet.c b/src/packet.c
>> index fbd94f1..2a22dbc 100644
>> --- a/src/packet.c
>> +++ b/src/packet.c
>> @@ -47,6 +47,12 @@ static int packet_adjust(struct pkt_hdr *pkt)
>> case ARPHRD_ETHER:
>> case ARPHRD_LOOPBACK:
>> pkt_cast_hdrp(ethhdr, pkt, 0);
>> + if (pkt->pkt_protocol == ETH_P_IP)
>> + pkt_cast_hdrp(iphdr, pkt, ETH_HLEN);
>> +
>> + if (pkt->pkt_protocol == ETH_P_IPV6)
>> + pkt_cast_hdrp(ip6_hdr, pkt, ETH_HLEN);
>> +
>> pkt->pkt_payload = pkt->pkt_buf;
>> pkt->pkt_payload += ETH_HLEN;
>> pkt->pkt_len -= ETH_HLEN;
>
> NAK.
>
> Tests for upper layer protocol should be moved after this switch and
> pkt_cast_hdrp() (better some new function) should use pkt->pkt_payload
> pointer. (You are mixing two protocol layers together - don't do this!)
I'm not sure if I'm on the same page.
Those two if()s must be in every case statement of the switch() in
packet_adjust() function, because IP or IPv6 can be seen not only on
loopback or ethernet but also on other devices e.g. on FDDI or else. So
it's better to move initializing of iphdr and ip6_hdr pointers to the new
function but then we must use the pkt->pkt_payload pointer. And we have
a problem when we are changing pkt->pkt_protocol (e.g. when tunneling
IPv6 in IP).
I made a patch (on top of this series) which takes care of it (see the
attached patch).
#define pkt_cast_hdrp(hdr, pkt, off) \
do { \
pkt->hdr = (struct hdr *) pkt->pkt_buf + off; \
} while (0)
intent to be generic macro, which can cast any type of net header
with offset to raw packet. I think, it pretty neat. _pkt_cast_hdrp()
But you can't mix pointers to layer2 protocols with pointers to layer3
protocols. Layer2 protocols start at pkt->pkt_buf, layer3 protocols
start at pkt->pkt_payload. See the attached patch.
also should be moved to packet.c, it's pretty dangerous and we
should
never us it outside of packet.c. I agree, that I miss extra VLAN_LEN
(pitty, that define is under __KERNEL__ guards).
I don't know what you mean by this? (yes, in my patch I moved the #define
into packet.c)
Vita
diff --git a/src/packet.c b/src/packet.c
index 2a22dbc..bb72f47 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -20,6 +20,17 @@ packet.c - routines to open the raw socket, read socket data and
#include "packet.h"
#include "ipfrag.h"
+#define pkt_cast_hdrp(hdr, pkt, off) \
+ do { \
+ pkt->hdr = (struct hdr *) pkt->pkt_buf + off; \
+ } while (0)
+
+#define pkt_cast_hdrp_l3(hdr, pkt, off) \
+ do { \
+ pkt->hdr = (struct hdr *) pkt->pkt_payload + off; \
+ } while (0)
+
+
/* code taken from
http://www.faqs.org/rfcs/rfc1071.html. See section 4.1 "C"
*/
static int in_cksum(u_short * addr, int len)
{
@@ -47,12 +58,6 @@ static int packet_adjust(struct pkt_hdr *pkt)
case ARPHRD_ETHER:
case ARPHRD_LOOPBACK:
pkt_cast_hdrp(ethhdr, pkt, 0);
- if (pkt->pkt_protocol == ETH_P_IP)
- pkt_cast_hdrp(iphdr, pkt, ETH_HLEN);
-
- if (pkt->pkt_protocol == ETH_P_IPV6)
- pkt_cast_hdrp(ip6_hdr, pkt, ETH_HLEN);
-
pkt->pkt_payload = pkt->pkt_buf;
pkt->pkt_payload += ETH_HLEN;
pkt->pkt_len -= ETH_HLEN;
@@ -98,6 +103,26 @@ static int packet_adjust(struct pkt_hdr *pkt)
return retval;
}
+/* initialize all layer3 protocol pointers (we need to initialize all
+ * of them, because of case we change pkt->pkt_protocol) */
+static void packet_set_l3_hdrp(struct pkt_hdr *pkt)
+{
+ switch (pkt->pkt_protocol) {
+ case ETH_P_IP:
+ pkt_cast_hdrp_l3(iphdr, pkt, 0);
+ pkt->ip6_hdr = NULL;
+ break;
+ case ETH_P_IPV6:
+ pkt->iphdr = NULL;
+ pkt_cast_hdrp_l3(ip6_hdr, pkt, 0);
+ break;
+ default:
+ pkt->iphdr = NULL;
+ pkt->ip6_hdr = NULL;
+ break;
+ }
+}
+
/* IPTraf input function; reads both keystrokes and network packets. */
int packet_get(int fd, struct pkt_hdr *pkt, int *ch, WINDOW *win)
{
@@ -161,8 +186,9 @@ int packet_process(struct pkt_hdr *pkt, unsigned int *total_br,
if (packet_adjust(pkt) != 0)
return INVALID_PACKET;
-again: if (pkt->pkt_protocol == ETH_P_IP) {
- struct iphdr *ip;
+again: packet_set_l3_hdrp(pkt);
+ if (pkt->pkt_protocol == ETH_P_IP) {
+ struct iphdr *ip = pkt->iphdr;
int hdr_check;
register int ip_checksum;
register int iphlen;
@@ -172,7 +198,6 @@ again: if (pkt->pkt_protocol == ETH_P_IP) {
* At this point, we're now processing IP packets. Start by getting
* IP header and length.
*/
- ip = (struct iphdr *) (pkt->pkt_payload);
iphlen = ip->ihl * 4;
/*
@@ -266,7 +291,7 @@ again: if (pkt->pkt_protocol == ETH_P_IP) {
} else if (pkt->pkt_protocol == ETH_P_IPV6) {
struct tcphdr *tcp;
struct udphdr *udp;
- struct ip6_hdr *ip6 = (struct ip6_hdr *) pkt->pkt_payload;
+ struct ip6_hdr *ip6 = pkt->ip6_hdr;
char *ip_payload = (char *) ip6 + 40;
//TODO: Filter packets
diff --git a/src/packet.h b/src/packet.h
index 15296a1..17e427e 100644
--- a/src/packet.h
+++ b/src/packet.h
@@ -40,12 +40,6 @@ struct pkt_hdr {
char pkt_buf[MAX_PACKET_SIZE];
};
-#define pkt_cast_hdrp(hdr, pkt, off) \
- do { \
- pkt->hdr = (struct hdr *) pkt->pkt_buf + off; \
- } while (0)
-
-
#define PACKET_INIT(packet) \
struct pkt_hdr packet = { \
.pkt_bufsize = MAX_PACKET_SIZE, \