Add ARP Responder module

Address code review comments

Author: GalSagie

core/modules/arp_responder.cc

@@ -19,9 +19,12 @@ CommandResponse ArpResponder::CommandAdd(const bess::pb::ArpResponderArg &arg) {
     return CommandFailure(EINVAL, "Invalid IP Address: %s", arg.ip().c_str());
   }
 
-  entry.mac_addr.FromString(arg.mac_addr());
+  if (!entry.mac_addr.FromString(arg.mac_addr())) {
+	  return CommandFailure(EINVAL, "Invalid MAC Address: %s", arg.mac_addr().c_str());
+  }
+
   entry.ip_addr = ip_addr;
-  entries[ip_addr] = entry;
+  entries_[ip_addr] = entry;
   return CommandSuccess();
 }
 
@@ -44,27 +47,27 @@ void ArpResponder::ProcessBatch(bess::PacketBatch *batch) {
 
     Arp *arp = reinterpret_cast<Arp *>(eth + 1);
     if (arp->opcode == be16_t(Arp::Opcode::kRequest)) {
-      // TODO When learn is added, learn SRC MAC here
+      // TODO(galsagie) When learn is added, learn SRC MAC here
 
       // Try to find target IP in cache, if exists convert request to reply
-      auto it = entries.find(arp->target_ip_addr);
-      if (it != entries.end()) {
-        struct arp_entry *entry = &it->second;
+      auto it = entries_.find(arp->target_ip_addr);
+      if (it != entries_.end()) {
+    	const struct arp_entry &entry = it->second;
         arp->opcode = be16_t(Arp::Opcode::kReply);
 
         eth->dst_addr = eth->src_addr;
-        eth->src_addr = entry->mac_addr;
+        eth->src_addr = entry.mac_addr;
 
         arp->target_hw_addr = arp->sender_hw_addr;
-        arp->sender_hw_addr = entry->mac_addr;
+        arp->sender_hw_addr = entry.mac_addr;
 
         arp->target_ip_addr = arp->sender_ip_addr;
-        arp->sender_ip_addr = entry->ip_addr;
+        arp->sender_ip_addr = entry.ip_addr;
       }
     } else if (arp->opcode == be16_t(Arp::Opcode::kReply)) {
-      // TODO When learn is added, learn SRC MAC here
+      // TODO(galsagie) When learn is added, learn SRC MAC here
     } else {
-      // TODO Other opcodes are not handled yet.
+      // TODO(galsagie) Other opcodes are not handled yet.
     }
   }
 

core/modules/arp_responder.h

@@ -12,12 +12,17 @@
 using bess::utils::Ethernet;
 using bess::utils::be32_t;
 
+// ARP cache entry struct which keeps mapping between IP and MAC
 struct arp_entry {
   Ethernet::Address mac_addr;
   be32_t ip_addr;
+  // timestamp used to expire cache entries
   uint64_t time;
 };
 
+// ARP Responder module
+// Answer ARP requests from an internal configurable cache
+// Currently drops non ARP packets
 class ArpResponder final : public Module {
  public:
   static const gate_idx_t kNumIGates = 1;
@@ -30,7 +35,8 @@ class ArpResponder final : public Module {
   CommandResponse CommandAdd(const bess::pb::ArpResponderArg &arg);
 
  private:
-  std::map<be32_t, arp_entry> entries;
+  // Mapping between IP (key) and its ARP entry (MAC Address)
+  std::map<be32_t, struct arp_entry> entries_;
 };
 
 #endif  // BESS_MODULES_ARP_RESPONDER_H_