lkl: lkl_add_arp_entry

[liuyuan10]

Provides API support to add permanent arp entries to lkl so outsiders
don't need to reply arp request.

Signed-off-by: Yuan Liu liuyuan@google.com

---

This change is

[ghost]

LGTM, I'll wait for Hajime to take a look as well before merging.

[thehajime]

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

Documentation/lkl.txt, line 196 [r1] (raw file):

  Add a list of arp permanent entries in the form of "ip|mac;ip|mac;..."

 $ LKL_HIJACK_NET_ARP="192.168.13.100|12:34:56:78:9a:bc;192.168.13.101|12:34:56:78:9a:be" lkl-hijack.sh ip address show

I think ip neighbor show command would be better in this case, no ?

---

tools/lkl/include/lkl.h, line 305 [r1] (raw file):

/**
 * lkl_add_arp_entry - add an permanent arp entry

an => a

---

tools/lkl/lib/net.c, line 164 [r1] (raw file):

struct lkl_arpreq {
  struct lkl_sockaddr_in arp_pa;      /* protocol address */
  struct lkl_sockaddr_in arp_ha;      /* hardware address */

arp_ha should be struct sockaddr, not with struct sockaddr_in ?

http://lxr.free-electrons.com/source/include/uapi/linux/if_arp.h#L113

---

tools/lkl/lib/net.c, line 167 [r1] (raw file):

  int             arp_flags;   /* flags */
  struct lkl_sockaddr_in arp_netmask; /* netmask of protocol address */
  char            arp_dev[16];

IFNAMSIZ instead of 16 ?

---

Comments from Reviewable

[thehajime]

@liuyuan10 thanks for the patch. overall the patch is good and useful and I agree with this if my minor comments are addressed.

[liuyuan10]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

Documentation/lkl.txt, line 196 [r1] (raw file):

Previously, thehajime (Hajime Tazaki) wrote…

I think ip neighbor show command would be better in this case, no ?

Done.

---

tools/lkl/include/lkl.h, line 305 [r1] (raw file):

Previously, thehajime (Hajime Tazaki) wrote…

an => a

Done.

---

tools/lkl/lib/net.c, line 164 [r1] (raw file):

Previously, thehajime (Hajime Tazaki) wrote…

arp_ha should be struct sockaddr, not with struct sockaddr_in ?

http://lxr.free-electrons.com/source/include/uapi/linux/if_arp.h#L113

The lkl_sockaddr has size of 128 which is different sockaddr.

asm/syscall.h:
#define lkl_sockaddr __lkl__kernel_sockaddr_storage

---

tools/lkl/lib/net.c, line 167 [r1] (raw file):

Previously, thehajime (Hajime Tazaki) wrote…

IFNAMSIZ instead of 16 ?

Done.

---

Comments from Reviewable

[thehajime]

Reviewed 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

tools/lkl/lib/net.c, line 164 [r1] (raw file):

Previously, liuyuan10 (Yuan Liu) wrote…

The lkl_sockaddr has size of 128 which is different sockaddr.

asm/syscall.h:
#define lkl_sockaddr __lkl__kernel_sockaddr_storage

hmm, okay if @opurdila is fine.

---

Comments from Reviewable

[liuyuan10]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

tools/lkl/lib/net.c, line 164 [r1] (raw file):

Previously, thehajime (Hajime Tazaki) wrote…

hmm, okay if @opurdila is fine.

@opurdila how do you think about it?

---

Comments from Reviewable

[ghost]

tools/lkl/lib/net.c, line 164 [r1] (raw file):

Previously, liuyuan10 (Yuan Liu) wrote…

@opurdila how do you think about it?

I am not sure about it, I am trying to figure out why did I use __lkl_kernel_sockaddr_storage. I know I added it to fix a build error after I pulled in 4.6-rc6 but now it seems incompatible with the if_arp stuff.

I am going to need the week-end to figure it out, but if this is urgent I can merge it and log an issue to fix after. Please let me know how do you prefer to proceed.

---

Comments from Reviewable

[liuyuan10]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

tools/lkl/lib/net.c, line 164 [r1] (raw file):

Previously, opurdila (Octavian Purdila) wrote…

I am not sure about it, I am trying to figure out why did I use __lkl_kernel_sockaddr_storage. I know I added it to fix a build error after I pulled in 4.6-rc6 but now it seems incompatible with the if_arp stuff.

I am going to need the week-end to figure it out, but if this is urgent I can merge it and log an issue to fix after. Please let me know how do you prefer to proceed.

Didn't realize there might be problem behind. I can wait. thanks

---

Comments from Reviewable

[liuyuan10]

Fixed after merging of #168

[ghost]

Resolved conflict and merged. Thanks @liuyuan10 !